Thanks Stephen for your eye on review, would collect other comment and refine it better in next version.
Best regards, Jeff Guo -----Original Message----- From: Stephen Hemminger [mailto:step...@networkplumber.org] Sent: Thursday, November 2, 2017 5:42 AM To: Guo, Jia <jia....@intel.com> Cc: Richardson, Bruce <bruce.richard...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>; gaetan.ri...@6wind.com; tho...@monjalon.net; Ananyev, Konstantin <konstantin.anan...@intel.com>; jblu...@infradead.org; shreyansh.j...@nxp.com; Wu, Jingjing <jingjing...@intel.com>; dev@dpdk.org; Zhang, Helin <helin.zh...@intel.com> Subject: Re: [PATCH v6 1/2] eal: add uevent monitor for hot plug On Thu, 2 Nov 2017 04:16:44 +0800 Jeff Guo <jia....@intel.com> wrote: > + > +static int > +dev_uev_parse(const char *buf, struct rte_eal_uevent *event) { > + char action[RTE_EAL_UEVENT_MSG_LEN]; > + char subsystem[RTE_EAL_UEVENT_MSG_LEN]; > + char dev_path[RTE_EAL_UEVENT_MSG_LEN]; > + char pci_slot_name[RTE_EAL_UEVENT_MSG_LEN]; > + int i = 0; > + > + memset(action, 0, RTE_EAL_UEVENT_MSG_LEN); > + memset(subsystem, 0, RTE_EAL_UEVENT_MSG_LEN); > + memset(dev_path, 0, RTE_EAL_UEVENT_MSG_LEN); > + memset(pci_slot_name, 0, RTE_EAL_UEVENT_MSG_LEN); > + > + while (i < RTE_EAL_UEVENT_MSG_LEN) { Might be simpler, safer, clearer to use rte_strsplit here. And then have a table of fields rather than open coding the parsing. > + for (; i < RTE_EAL_UEVENT_MSG_LEN; i++) { > + if (*buf) > + break; > + buf++; > + } > + if (!strncmp(buf, "libudev", 7)) { > + buf += 7; > + i += 7; > + event->group = UEV_MONITOR_UDEV; > + } > + if (!strncmp(buf, "ACTION=", 7)) { > + buf += 7; > + i += 7; > + snprintf(action, sizeof(action), "%s", buf); > + } else if (!strncmp(buf, "DEVPATH=", 8)) { > + buf += 8; > + i += 8; > + snprintf(dev_path, sizeof(dev_path), "%s", buf); > + } else if (!strncmp(buf, "SUBSYSTEM=", 10)) { > + buf += 10; > + i += 10; > + snprintf(subsystem, sizeof(subsystem), "%s", buf); > + } else if (!strncmp(buf, "PCI_SLOT_NAME=", 14)) { > + buf += 14; > + i += 14; > + snprintf(pci_slot_name, sizeof(subsystem), "%s", buf); > + } > + for (; i < RTE_EAL_UEVENT_MSG_LEN; i++) { > + if (*buf == '\0') > + break; > + buf++; > + } > + } > + > + if (!strncmp(subsystem, "pci", 3)) > + event->subsystem = UEV_SUBSYSTEM_PCI; > + if (!strncmp(action, "add", 3)) > + event->type = RTE_EAL_DEV_EVENT_ADD; > + if (!strncmp(action, "remove", 6)) > + event->type = RTE_EAL_DEV_EVENT_REMOVE; > + event->devname = pci_slot_name; > + > + return 0; Function always returns 0, why is it not void?
