| Message ID | 47646199-1784-f1ac-23ae-eedaf3a24c95@gentoo.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Jul 31, 2017, at 12:25 AM, Joshua Kinard <kumba@gentoo.org> wrote: > > As annotated in the bzero(3) man page, bzero() was marked as LEGACY in > POSIX.1-2001 and removed in POSIX.1-2008, and should be replaced with > memset() calls to write zeros to a memory region. The attached patch > replaces two bzero() calls and one __bzero() call in libtirpc with > equivalent memset() calls. The latter replacement fixes a compile error > under uclibc-ng, which lacks a definition for __bzero() OK. Nits below. Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > Signed-off-by: Joshua Kinard <kumba@gentoo.org> > --- > > src/auth_time.c | 2 +- > src/des_impl.c | 2 +- > src/svc_auth_des.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/auth_time.c b/src/auth_time.c > index 7f83ab4..210d251 100644 > --- a/src/auth_time.c > +++ b/src/auth_time.c > @@ -317,7 +317,7 @@ __rpc_get_time_offset(td, srv, thost, uaddr, netid) > sprintf(ipuaddr, "%d.%d.%d.%d.0.111", a1, a2, a3, a4); > useua = &ipuaddr[0]; > > - bzero((char *)&sin, sizeof(sin)); > + memset((char *)&sin, 0, sizeof(sin)); The type cast is likely to be unnecessary, and could be removed in this patch. > if (uaddr_to_sockaddr(useua, &sin)) { > msg("unable to translate uaddr to sockaddr."); > if (needfree) > diff --git a/src/des_impl.c b/src/des_impl.c > index 9dbccaf..15bec2a 100644 > --- a/src/des_impl.c > +++ b/src/des_impl.c > @@ -588,7 +588,7 @@ _des_crypt (char *buf, unsigned len, struct desparams *desp) > } > tin0 = tin1 = tout0 = tout1 = xor0 = xor1 = 0; > tbuf[0] = tbuf[1] = 0; > - __bzero (schedule, sizeof (schedule)); > + memset (schedule, 0, sizeof (schedule)); You should fix the white space damage here (space before left paren, x2). > return (1); > } > diff --git a/src/svc_auth_des.c b/src/svc_auth_des.c > index 2e90146..64a7682 100644 > --- a/src/svc_auth_des.c > +++ b/src/svc_auth_des.c > @@ -356,7 +356,7 @@ cache_init() > > authdes_cache = (struct cache_entry *) > mem_alloc(sizeof(struct cache_entry) * AUTHDES_CACHESZ); > - bzero((char *)authdes_cache, > + memset((char *)authdes_cache, 0, > sizeof(struct cache_entry) * AUTHDES_CACHESZ); Type cast is unnecessary. > authdes_lru = (short *)mem_alloc(sizeof(short) * AUTHDES_CACHESZ); > -- Chuck Lever chucklever@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/31/2017 10:35, Chuck Lever wrote: > >> On Jul 31, 2017, at 12:25 AM, Joshua Kinard <kumba@gentoo.org> wrote: >> >> As annotated in the bzero(3) man page, bzero() was marked as LEGACY in >> POSIX.1-2001 and removed in POSIX.1-2008, and should be replaced with >> memset() calls to write zeros to a memory region. The attached patch >> replaces two bzero() calls and one __bzero() call in libtirpc with >> equivalent memset() calls. The latter replacement fixes a compile error >> under uclibc-ng, which lacks a definition for __bzero() > > OK. Nits below. > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > > >> Signed-off-by: Joshua Kinard <kumba@gentoo.org> >> --- >> >> src/auth_time.c | 2 +- >> src/des_impl.c | 2 +- >> src/svc_auth_des.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/auth_time.c b/src/auth_time.c >> index 7f83ab4..210d251 100644 >> --- a/src/auth_time.c >> +++ b/src/auth_time.c >> @@ -317,7 +317,7 @@ __rpc_get_time_offset(td, srv, thost, uaddr, netid) >> sprintf(ipuaddr, "%d.%d.%d.%d.0.111", a1, a2, a3, a4); >> useua = &ipuaddr[0]; >> >> - bzero((char *)&sin, sizeof(sin)); >> + memset((char *)&sin, 0, sizeof(sin)); > > The type cast is likely to be unnecessary, and > could be removed in this patch. Wasn't trying to solve all of the problems at once. Figured it was better to go for the straight replace. I'll update the patch and resend with this change. >> if (uaddr_to_sockaddr(useua, &sin)) { >> msg("unable to translate uaddr to sockaddr."); >> if (needfree) >> diff --git a/src/des_impl.c b/src/des_impl.c >> index 9dbccaf..15bec2a 100644 >> --- a/src/des_impl.c >> +++ b/src/des_impl.c >> @@ -588,7 +588,7 @@ _des_crypt (char *buf, unsigned len, struct desparams *desp) >> } >> tin0 = tin1 = tout0 = tout1 = xor0 = xor1 = 0; >> tbuf[0] = tbuf[1] = 0; >> - __bzero (schedule, sizeof (schedule)); >> + memset (schedule, 0, sizeof (schedule)); > > You should fix the white space damage here > (space before left paren, x2). IMHO, I think that, for now, the whitespace damage should be left as-is. Similar damage is prolific in the entire source file, so it should be fixed all at once in a separate patch to keep the change history consistent. I can send something in for just this file later on, but after this patch is applied. >> return (1); >> } >> diff --git a/src/svc_auth_des.c b/src/svc_auth_des.c >> index 2e90146..64a7682 100644 >> --- a/src/svc_auth_des.c >> +++ b/src/svc_auth_des.c >> @@ -356,7 +356,7 @@ cache_init() >> >> authdes_cache = (struct cache_entry *) >> mem_alloc(sizeof(struct cache_entry) * AUTHDES_CACHESZ); >> - bzero((char *)authdes_cache, >> + memset((char *)authdes_cache, 0, >> sizeof(struct cache_entry) * AUTHDES_CACHESZ); > > Type cast is unnecessary. Will be updated in a v2 later on. Thanks for the review!
On 07/31/2017 11:05 AM, Joshua Kinard wrote: > On 07/31/2017 10:35, Chuck Lever wrote: >> >>> On Jul 31, 2017, at 12:25 AM, Joshua Kinard <kumba@gentoo.org> wrote: >>> >>> As annotated in the bzero(3) man page, bzero() was marked as LEGACY in >>> POSIX.1-2001 and removed in POSIX.1-2008, and should be replaced with >>> memset() calls to write zeros to a memory region. The attached patch >>> replaces two bzero() calls and one __bzero() call in libtirpc with >>> equivalent memset() calls. The latter replacement fixes a compile error >>> under uclibc-ng, which lacks a definition for __bzero() >> >> OK. Nits below. >> >> Reviewed-by: Chuck Lever <chuck.lever@oracle.com> >> >> >>> Signed-off-by: Joshua Kinard <kumba@gentoo.org> >>> --- >>> >>> src/auth_time.c | 2 +- >>> src/des_impl.c | 2 +- >>> src/svc_auth_des.c | 2 +- >>> 3 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/auth_time.c b/src/auth_time.c >>> index 7f83ab4..210d251 100644 >>> --- a/src/auth_time.c >>> +++ b/src/auth_time.c >>> @@ -317,7 +317,7 @@ __rpc_get_time_offset(td, srv, thost, uaddr, netid) >>> sprintf(ipuaddr, "%d.%d.%d.%d.0.111", a1, a2, a3, a4); >>> useua = &ipuaddr[0]; >>> >>> - bzero((char *)&sin, sizeof(sin)); >>> + memset((char *)&sin, 0, sizeof(sin)); >> >> The type cast is likely to be unnecessary, and >> could be removed in this patch. > > Wasn't trying to solve all of the problems at once. Figured it was better to > go for the straight replace. I'll update the patch and resend with this change. > > > >>> if (uaddr_to_sockaddr(useua, &sin)) { >>> msg("unable to translate uaddr to sockaddr."); >>> if (needfree) >>> diff --git a/src/des_impl.c b/src/des_impl.c >>> index 9dbccaf..15bec2a 100644 >>> --- a/src/des_impl.c >>> +++ b/src/des_impl.c >>> @@ -588,7 +588,7 @@ _des_crypt (char *buf, unsigned len, struct desparams *desp) >>> } >>> tin0 = tin1 = tout0 = tout1 = xor0 = xor1 = 0; >>> tbuf[0] = tbuf[1] = 0; >>> - __bzero (schedule, sizeof (schedule)); >>> + memset (schedule, 0, sizeof (schedule)); >> >> You should fix the white space damage here >> (space before left paren, x2). > > IMHO, I think that, for now, the whitespace damage should be left as-is. > Similar damage is prolific in the entire source file, so it should be fixed all > at once in a separate patch to keep the change history consistent. I can send > something in for just this file later on, but after this patch is applied. I agree... leave as is.. > > >>> return (1); >>> } >>> diff --git a/src/svc_auth_des.c b/src/svc_auth_des.c >>> index 2e90146..64a7682 100644 >>> --- a/src/svc_auth_des.c >>> +++ b/src/svc_auth_des.c >>> @@ -356,7 +356,7 @@ cache_init() >>> >>> authdes_cache = (struct cache_entry *) >>> mem_alloc(sizeof(struct cache_entry) * AUTHDES_CACHESZ); >>> - bzero((char *)authdes_cache, >>> + memset((char *)authdes_cache, 0, >>> sizeof(struct cache_entry) * AUTHDES_CACHESZ); >> >> Type cast is unnecessary. > > Will be updated in a v2 later on. Sounds like a plan... better sooner vs later.. ;-) steved. > > Thanks for the review! > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/auth_time.c b/src/auth_time.c index 7f83ab4..210d251 100644 --- a/src/auth_time.c +++ b/src/auth_time.c @@ -317,7 +317,7 @@ __rpc_get_time_offset(td, srv, thost, uaddr, netid) sprintf(ipuaddr, "%d.%d.%d.%d.0.111", a1, a2, a3, a4); useua = &ipuaddr[0]; - bzero((char *)&sin, sizeof(sin)); + memset((char *)&sin, 0, sizeof(sin)); if (uaddr_to_sockaddr(useua, &sin)) { msg("unable to translate uaddr to sockaddr."); if (needfree) diff --git a/src/des_impl.c b/src/des_impl.c index 9dbccaf..15bec2a 100644 --- a/src/des_impl.c +++ b/src/des_impl.c @@ -588,7 +588,7 @@ _des_crypt (char *buf, unsigned len, struct desparams *desp) } tin0 = tin1 = tout0 = tout1 = xor0 = xor1 = 0; tbuf[0] = tbuf[1] = 0; - __bzero (schedule, sizeof (schedule)); + memset (schedule, 0, sizeof (schedule)); return (1); } diff --git a/src/svc_auth_des.c b/src/svc_auth_des.c index 2e90146..64a7682 100644 --- a/src/svc_auth_des.c +++ b/src/svc_auth_des.c @@ -356,7 +356,7 @@ cache_init() authdes_cache = (struct cache_entry *) mem_alloc(sizeof(struct cache_entry) * AUTHDES_CACHESZ); - bzero((char *)authdes_cache, + memset((char *)authdes_cache, 0, sizeof(struct cache_entry) * AUTHDES_CACHESZ); authdes_lru = (short *)mem_alloc(sizeof(short) * AUTHDES_CACHESZ);
As annotated in the bzero(3) man page, bzero() was marked as LEGACY in POSIX.1-2001 and removed in POSIX.1-2008, and should be replaced with memset() calls to write zeros to a memory region. The attached patch replaces two bzero() calls and one __bzero() call in libtirpc with equivalent memset() calls. The latter replacement fixes a compile error under uclibc-ng, which lacks a definition for __bzero() Signed-off-by: Joshua Kinard <kumba@gentoo.org> --- src/auth_time.c | 2 +- src/des_impl.c | 2 +- src/svc_auth_des.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html