diff mbox series

[v3] replace sprintf with stpcpy to avoid GCC warning [BZ#28439]

Message ID d4e55042-0138-c2ff-a1ba-4b782030e884@gmail.com
State Committed
Commit eb73b87897798de981dbbf019aa957045d768adb
Headers
Series [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

Martin Sebor Oct. 9, 2021, 11:43 p.m. UTC 
  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

Florian Weimer Oct. 10, 2021, 1:53 p.m. UTC | #1 
* 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).
Martin Sebor Oct. 11, 2021, 3:43 p.m. UTC | #2 
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
diff mbox series

Patch

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,