On 2014/10/29 11:44, Matthew Hall wrote: > On Wed, Oct 29, 2014 at 03:27:58AM +0000, Qiu, Michael wrote: >> I just saw one return path with value '0', and no any other place >> return a negative value, so it is better to be designed as one >> non-return function, >> >> +void >> +rte_eal_hugepage_free(void) >> +{ >> + struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl; >> + unsigned i; >> + unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles; >> + >> + RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles); >> + >> + for (i = 0; i < nr_hugefiles; i++) { >> + unlink(hugepg_tbl[i].filepath); >> + hugepg_tbl[i].orig_va = NULL; >> + } >> +} >> + >> >> Thanks, >> Michael > > Actually, I don't think that's quite right. > > http://linux.die.net/man/2/unlink > > "On success, zero is returned. On error, -1 is returned, and errno is set > appropriately." So it should be returning an error, and logging a message for > a file it cannot unlink or people will be surprised with weird failures. > > It also had some minor typos / English in the comments but we can fix that > too. > > Matthew. > >
Thank you Michael & Matthew I will fix it. :) -- Regards, Haifeng