On Sun, Sep 27, 2020 at 10:08:49PM -0600, Alan Somers wrote: > On Sun, Sep 27, 2020 at 5:15 PM Konstantin Belousov <kostik...@gmail.com> > wrote: > > > On Sun, Sep 27, 2020 at 10:26:41PM +0000, Alan Somers wrote: > > > Author: asomers > > > Date: Sun Sep 27 22:26:41 2020 > > > New Revision: 366207 > > > URL: https://svnweb.freebsd.org/changeset/base/366207 > > > > > > Log: > > > Misc compiler warning fixes in lib/libc > > > > > > Reviewed by: kevans, imp > > > MFC after: 2 weeks > > > Differential Revision: https://reviews.freebsd.org/D26534 > > > > > > Modified: > > > head/lib/libc/gen/auxv.c > > > head/lib/libc/gen/basename_compat.c > > > head/lib/libc/gen/crypt.c > > > head/lib/libc/gen/dirname_compat.c > > > head/lib/libc/gen/fts-compat.c > > > head/lib/libc/gen/ftw-compat11.c > > > head/lib/libc/gen/getentropy.c > > > > > > Modified: head/lib/libc/gen/auxv.c > > > > > ============================================================================== > > > --- head/lib/libc/gen/auxv.c Sun Sep 27 21:43:19 2020 (r366206) > > > +++ head/lib/libc/gen/auxv.c Sun Sep 27 22:26:41 2020 (r366207) > > > @@ -67,7 +67,8 @@ __init_elf_aux_vector(void) > > > } > > > > > > static pthread_once_t aux_once = PTHREAD_ONCE_INIT; > > > -static int pagesize, osreldate, canary_len, ncpus, pagesizes_len, > > bsdflags; > > > +static int pagesize, osreldate, ncpus, bsdflags; > > > +static size_t canary_len, pagesizes_len; > > > static int hwcap_present, hwcap2_present; > > > static char *canary, *pagesizes, *execpath; > > > static void *ps_strings, *timekeep; > > > @@ -245,16 +246,21 @@ int > > > _elf_aux_info(int aux, void *buf, int buflen) > > > { > > > int res; > > > + size_t buflen_; > > > > > > __init_elf_aux_vector(); > > > if (__elf_aux_vector == NULL) > > > return (ENOSYS); > > > _once(&aux_once, init_aux); > > > > > > + if (buflen < 0) > > > + return (EINVAL); > > > + buflen_ = (size_t)buflen; > > > + > > > switch (aux) { > > > case AT_CANARY: > > > - if (canary != NULL && canary_len >= buflen) { > > > - memcpy(buf, canary, buflen); > > > + if (canary != NULL && canary_len >= buflen_) { > > > + memcpy(buf, canary, buflen_); > > > memset(canary, 0, canary_len); > > > canary = NULL; > > > res = 0; > > > @@ -267,35 +273,35 @@ _elf_aux_info(int aux, void *buf, int buflen) > > > else if (buf == NULL) > > > res = EINVAL; > > > else { > > > - if (strlcpy(buf, execpath, buflen) >= buflen) > > > + if (strlcpy(buf, execpath, buflen_) >= buflen_) > > > res = EINVAL; > > > else > > > res = 0; > > > } > > > break; > > > case AT_HWCAP: > > > - if (hwcap_present && buflen == sizeof(u_long)) { > > > + if (hwcap_present && buflen_ == sizeof(u_long)) { > > > *(u_long *)buf = hwcap; > > > res = 0; > > > } else > > > res = ENOENT; > > > break; > > > case AT_HWCAP2: > > > - if (hwcap2_present && buflen == sizeof(u_long)) { > > > + if (hwcap2_present && buflen_ == sizeof(u_long)) { > > > *(u_long *)buf = hwcap2; > > > res = 0; > > > } else > > > res = ENOENT; > > > break; > > > case AT_PAGESIZES: > > > - if (pagesizes != NULL && pagesizes_len >= buflen) { > > > - memcpy(buf, pagesizes, buflen); > > > + if (pagesizes != NULL && pagesizes_len >= buflen_) { > > > + memcpy(buf, pagesizes, buflen_); > > > res = 0; > > > } else > > > res = ENOENT; > > > break; > > > case AT_PAGESZ: > > > - if (buflen == sizeof(int)) { > > > + if (buflen_ == sizeof(int)) { > > > if (pagesize != 0) { > > > *(int *)buf = pagesize; > > > res = 0; > > > @@ -305,7 +311,7 @@ _elf_aux_info(int aux, void *buf, int buflen) > > > res = EINVAL; > > > break; > > > case AT_OSRELDATE: > > > - if (buflen == sizeof(int)) { > > > + if (buflen_ == sizeof(int)) { > > > if (osreldate != 0) { > > > *(int *)buf = osreldate; > > > res = 0; > > > @@ -315,7 +321,7 @@ _elf_aux_info(int aux, void *buf, int buflen) > > > res = EINVAL; > > > break; > > > case AT_NCPUS: > > > - if (buflen == sizeof(int)) { > > > + if (buflen_ == sizeof(int)) { > > > if (ncpus != 0) { > > > *(int *)buf = ncpus; > > > res = 0; > > > @@ -325,7 +331,7 @@ _elf_aux_info(int aux, void *buf, int buflen) > > > res = EINVAL; > > > break; > > > case AT_TIMEKEEP: > > > - if (buflen == sizeof(void *)) { > > > + if (buflen_ == sizeof(void *)) { > > > if (timekeep != NULL) { > > > *(void **)buf = timekeep; > > > res = 0; > > > @@ -335,14 +341,14 @@ _elf_aux_info(int aux, void *buf, int buflen) > > > res = EINVAL; > > > break; > > > case AT_BSDFLAGS: > > > - if (buflen == sizeof(int)) { > > > + if (buflen_ == sizeof(int)) { > > > *(int *)buf = bsdflags; > > > res = 0; > > > } else > > > res = EINVAL; > > > break; > > > case AT_PS_STRINGS: > > > - if (buflen == sizeof(void *)) { > > > + if (buflen_ == sizeof(void *)) { > > > if (ps_strings != NULL) { > > > *(void **)buf = ps_strings; > > > res = 0; > > > > This is significant uglification of the code in the name of fixing > > pointless > > warnings. > > > > Warnings are NOT pointless. 99% of them are. But the only way to find the > 1% that aren't is to quell the 99% that are. Last week I wrote a bug that > fortunately got caught in code review. But it shouldn't have made it that > far. It should've been caught by the compiler, but libc is only built with > WARNS=2. This commit is one step towards raising libc's WARNs level to 3. > Only 334 files left to go. I specifically replied to the part of the change that I quoted. I did not discussed generic arguments WRT handling some warnings.
> > > > > > I suspect that you tried to shut down warning about comparision of integers > > of different size, int vs. sizeof() which has size_t result. Is there > > anything else ? > > > > Mostly the warnings were about comparisons of integers with different > signedness. Also there were some warnings about missing prototypes. In this fragment ? I think no, so only signess mismatch. > > > > > > All these values should be small integers, which is quite vividly > > illustrated by comparision with sizeof() of built-in types. If compiler > > cannot deduce that itself the warning should be forcibly disabled by > > flag instead of doing 'buflen_'. > > > > And canary_len/pagesizes_len do not need to take 8 bytes, their values > > never become greater than one hundred. > > > > Better to give it enough space that the compiler can check it statically, > rather than require runtime checks. Natural and proper type for small numerical values in C is int. Both external interface for the API, and internal implementation are built around this. Making internal details mismatch the external view on the interface make it much more risky then leaving warning that is mostly due to the compiler deficiency. Using size_t is not appropriate there. I prefer this warning to be shut down for the file with the compiler switch. If you are so inclined to shut it in the source code, please either change the type of unneeded variable to unsigned int (and also perhaps applying ScottL suggestion), or just do casts to int in comparisions. _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"