CFR: Exceedingly minor fixes to libc
Garrett Wollman
contains some very minor fixes to various parts of libc which were
found by the clang static analyzer. They fall into a few categories:
1) Bug fixes in very rare situations (mostly error-handling code that
has probably never been executed).
2) Dead store elimination.
3) Elimination of unused variables. (Or in a few cases, making use of
them.)
Some minor style problems were also fixed in the vicinity. There
should be no functional changes except in very unusual conditions.
-GAWollman
Jilles Tjoelker
> 2) Dead store elimination.
This looks mostly ok, with a few exceptions.
> [...]
> Index: gen/getpwent.c
> ===================================================================
> --- gen/getpwent.c (revision 199242)
> +++ gen/getpwent.c (working copy)
> @@ -257,22 +257,19 @@
> [...]
> @@ -1876,7 +1871,6 @@
> syslog(LOG_ERR,
> "getpwent memory allocation failure");
> *errnop = ENOMEM;
> - rv = NS_UNAVAIL;
> break;
> }
> st->compat = COMPAT_MODE_NAME;
I think this change hides the wrongness in the handling of malloc errors
while leaving it as broken as it is.
> [...]
> Index: net/getservent.c
> ===================================================================
> --- net/getservent.c (revision 199242)
> +++ net/getservent.c (working copy)
> @@ -476,11 +476,11 @@
> struct nis_state *st;
> int rv;
>
> - enum nss_lookup_type how;
> char *name;
> char *proto;
> int port;
>
> + enum nss_lookup_type how;
> struct servent *serv;
> char *buffer;
> size_t bufsize;
> @@ -491,7 +491,8 @@
>
> name = NULL;
> proto = NULL;
> - how = (enum nss_lookup_type)mdata;
> +
> + how = (intptr_t)mdata;
> switch (how) {
> case nss_lt_name:
> name = va_arg(ap, char *);
In what way is this an improvement?
> Index: posix1e/acl_delete_entry.c
> ===================================================================
> --- posix1e/acl_delete_entry.c (revision 199242)
> +++ posix1e/acl_delete_entry.c (working copy)
> @@ -89,20 +89,20 @@
> return (-1);
> }
>
> - if ((acl->ats_acl.acl_cnt < 1) ||
> - (acl->ats_acl.acl_cnt > ACL_MAX_ENTRIES)) {
> + if ((acl_int->acl_cnt < 1) ||
> + (acl_int->acl_cnt > ACL_MAX_ENTRIES)) {
> errno = EINVAL;
> return (-1);
> }
If you are changing this code anyway, consider fixing a style bug
(extraneous parentheses) here.
> - for (i = 0; i < acl->ats_acl.acl_cnt;) {
> - if (_entry_matches(&(acl->ats_acl.acl_entry[i]), entry_d)) {
> + for (i = 0; i < acl_int->acl_cnt;) {
> + if (_entry_matches(&(acl_int->acl_entry[i]), entry_d)) {
> /* ...shift the remaining entries... */
> - for (j = i; j < acl->ats_acl.acl_cnt - 1; ++j)
> - acl->ats_acl.acl_entry[j] =
> - acl->ats_acl.acl_entry[j+1];
> + for (j = i; j < acl_int->acl_cnt - 1; ++j)
> + acl_int->acl_entry[j] =
> + acl_int->acl_entry[j+1];
> /* ...drop the count and zero the unused entry... */
> - acl->ats_acl.acl_cnt--;
> - bzero(&acl->ats_acl.acl_entry[j],
> + acl_int->acl_cnt--;
> + bzero(&acl_int->acl_entry[j],
> sizeof(struct acl_entry));
> acl->ats_cur_entry = 0;
>
--
Jilles Tjoelker
_______________________________________________
freebsd...@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hacke...@freebsd.org"
Hajimu UMEMOTO
>>>>> On Fri, 13 Nov 2009 00:18:49 -0500
>>>>> Garrett Wollman <wol...@bimajority.org> said:
wollman> Index: inet/inet_cidr_pton.c
wollman> ===================================================================
wollman> --- inet/inet_cidr_pton.c (revision 199242)
wollman> +++ inet/inet_cidr_pton.c (working copy)
wollman> @@ -236,7 +236,6 @@
wollman> endp[- i] = colonp[n - i];
wollman> colonp[n - i] = 0;
wollman> }
wollman> - tp = endp;
wollman> }
wollman>
wollman> memcpy(dst, tmp, NS_IN6ADDRSZ);
Since this function is vendor import one from ISC, such cosmetic
change may cause problem during further import. So, you should send
this patch to ISC folks 1st.
Sincerely,
--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
u...@mahoroba.org ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/