[v3] replace sprintf with stpcpy to avoid GCC warning [BZ#28439]
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
On 10/9/21 3:57 PM, Paul Eggert wrote:
> On 10/9/21 1:56 PM, Martin Sebor via Libc-alpha wrote:
>> I would
>> have thought the strcpy approach preferable
>
> Me too. sprintf/snprintf/__snprintf is overkill here. Instead, I suggest
> something like the following (untested) patch, as it is easier for
> static analysis that would otherwise have to worry about the INT_MAX
> gorp that plagues the sprintf family.
>
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index 75b0e5f2f7..5d0a68dc81 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -610,7 +610,9 @@ __res_context_querydomain (struct resolv_context *ctx,
> RES_SET_H_ERRNO(statp, NO_RECOVERY);
> return (-1);
> }
> - sprintf(nbuf, "%s.%s", name, domain);
> + char *p = __stpcpy (nbuf, name);
> + *p++ = '.';
> + strcpy (p, domain);
> }
> return __res_context_query (ctx, longname, class, type, answer,
> anslen, answerp, answerp2, nanswerp2,
That works for me too. Attached is v3 with the above patch for
Patchwork to test. It builds and passes with no extra failures
for me on x86_64-linux.
I'll plan to commit this version sometime next week if it passes
and there's no further feedback. Florian, please pipe up if you
have any concerns with it.
Martin
Comments
* Martin Sebor:
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index 75b0e5f2f7..5d0a68dc81 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -610,7 +610,9 @@ __res_context_querydomain (struct resolv_context *ctx,
> RES_SET_H_ERRNO(statp, NO_RECOVERY);
> return (-1);
> }
> - sprintf(nbuf, "%s.%s", name, domain);
> + char *p = __stpcpy (nbuf, name);
> + *p++ = '.';
> + strcpy (p, domain);
> }
> return __res_context_query (ctx, longname, class, type, answer,
> anslen, answerp, answerp2, nanswerp2,
I don't see any problems with this patch, although I dislike the use
of stpcpy in general (mempcpy and stpcopy are both closely associated
with buggy code).
On 10/10/21 7:53 AM, Florian Weimer wrote:
> * Martin Sebor:
>
>> diff --git a/resolv/res_query.c b/resolv/res_query.c
>> index 75b0e5f2f7..5d0a68dc81 100644
>> --- a/resolv/res_query.c
>> +++ b/resolv/res_query.c
>> @@ -610,7 +610,9 @@ __res_context_querydomain (struct resolv_context *ctx,
>> RES_SET_H_ERRNO(statp, NO_RECOVERY);
>> return (-1);
>> }
>> - sprintf(nbuf, "%s.%s", name, domain);
>> + char *p = __stpcpy (nbuf, name);
>> + *p++ = '.';
>> + strcpy (p, domain);
>> }
>> return __res_context_query (ctx, longname, class, type, answer,
>> anslen, answerp, answerp2, nanswerp2,
>
> I don't see any problems with this patch, although I dislike the use
> of stpcpy in general (mempcpy and stpcopy are both closely associated
> with buggy code).
>
Okay, thanks. I've pushed the change in
eb73b87897798de981dbbf019aa957045d768adb.
Martin
@@ -610,7 +610,9 @@ __res_context_querydomain (struct resolv_context *ctx,
RES_SET_H_ERRNO(statp, NO_RECOVERY);
return (-1);
}
- sprintf(nbuf, "%s.%s", name, domain);
+ char *p = __stpcpy (nbuf, name);
+ *p++ = '.';
+ strcpy (p, domain);
}
return __res_context_query (ctx, longname, class, type, answer,
anslen, answerp, answerp2, nanswerp2,