2017-07-31 04:25:33

by Joshua Kinard

[permalink] [raw]
Subject: [PATCH] Replace bzero() calls with equivalent memset() calls

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 <[email protected]>
---

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));
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);



2017-07-31 14:35:37

by Chuck Lever

[permalink] [raw]
Subject: Re: [Libtirpc-devel] [PATCH] Replace bzero() calls with equivalent memset() calls


> On Jul 31, 2017, at 12:25 AM, Joshua Kinard <[email protected]> wrote:
>=20
> 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 <[email protected]>


> Signed-off-by: Joshua Kinard <[email protected]>
> ---
>=20
> src/auth_time.c | 2 +-
> src/des_impl.c | 2 +-
> src/svc_auth_des.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>=20
> 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 =3D &ipuaddr[0];
>=20
> - 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 =3D tin1 =3D tout0 =3D tout1 =3D xor0 =3D xor1 =3D 0;
> tbuf[0] =3D tbuf[1] =3D 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()
>=20
> authdes_cache =3D (struct cache_entry *)
> mem_alloc(sizeof(struct cache_entry) * AUTHDES_CACHESZ);=09=

> - bzero((char *)authdes_cache,=20
> + memset((char *)authdes_cache, 0,
> sizeof(struct cache_entry) * AUTHDES_CACHESZ);

Type cast is unnecessary.


> authdes_lru =3D (short *)mem_alloc(sizeof(short) * =
AUTHDES_CACHESZ);
>=20

--
Chuck Lever
[email protected]




2017-07-31 15:05:51

by Joshua Kinard

[permalink] [raw]
Subject: Re: [Libtirpc-devel] [PATCH] Replace bzero() calls with equivalent memset() calls

On 07/31/2017 10:35, Chuck Lever wrote:
>
>> On Jul 31, 2017, at 12:25 AM, Joshua Kinard <[email protected]> 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 <[email protected]>
>
>
>> Signed-off-by: Joshua Kinard <[email protected]>
>> ---
>>
>> 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!

--
Joshua Kinard
Gentoo/MIPS
[email protected]
6144R/F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943

"The past tempts us, the present confuses us, the future frightens us. And our
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

2017-07-31 15:15:32

by Steve Dickson

[permalink] [raw]
Subject: Re: [Libtirpc-devel] [PATCH] Replace bzero() calls with equivalent memset() calls



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 <[email protected]> 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 <[email protected]>
>>
>>
>>> Signed-off-by: Joshua Kinard <[email protected]>
>>> ---
>>>
>>> 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!
>