[PATCH] lib/util/util.c: move null-check before use

Herb Lewis hlewis at panasas.com
Thu Feb 25 23:23:29 UTC 2016


I'm confused as to why we do the initial assignment to len at all
in the declaration. It gets overwritten by the return value of
snprintf before it is ever used.

On 02/25/2016 03:12 PM, Jeremy Allison wrote:
> On Thu, Feb 25, 2016 at 05:06:41PM +0100, Aurélien Aptel wrote:
>> Hi,
>>
>> This patch moves the null pointer check *before* the pointer is used in
>> the strlen() call.
>>      
>> - still allocate the `fname` array on the stack
>> - still compiles under C90 definition/code mixing rules
>>
>> Please review&push
> A bit too 'clever' I'm afraid, although correct.
>
> I'd prefer splitting out so len is declared, than
> initialized after the check for !dir.
>
> Jeremy.
>
>> Thanks!
>>
>> -- 
>> Aurélien Aptel / SUSE Labs Samba Team
>> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
>> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
>> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
>> Nürnberg)
>>  From 675aa8de1876c026052248f5112db8a15562980c Mon Sep 17 00:00:00 2001
>> From: Aurelien Aptel <aaptel at suse.com>
>> Date: Thu, 25 Feb 2016 14:11:10 +0100
>> Subject: [PATCH 4/4] lib/util/util.c: move null-check before use
>>
>> moves a null pointer check *before* the pointer is used in
>> the strlen() call
>>
>> - still allocate the `fname` array on the stack
>> - still compiles under C90 definition/code mixing rules
>>
>> Signed-off-by: Aurelien Aptel <aaptel at suse.com>
>> ---
>>   lib/util/util.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/lib/util/util.c b/lib/util/util.c
>> index 03edd7f..683741c 100644
>> --- a/lib/util/util.c
>> +++ b/lib/util/util.c
>> @@ -66,15 +66,11 @@ _PUBLIC_ const char *tmpdir(void)
>>   **/
>>   int create_unlink_tmp(const char *dir)
>>   {
>> -	size_t len = strlen(dir);
>> +	size_t len = strlen(dir ? dir : (dir = tmpdir()));
>>   	char fname[len+25];
>>   	int fd;
>>   	mode_t mask;
>>   
>> -	if (!dir) {
>> -		dir = tmpdir();
>> -	}
>> -
>>   	len = snprintf(fname, sizeof(fname), "%s/listenerlock_XXXXXX", dir);
>>   	if (len >= sizeof(fname)) {
>>   		errno = ENOMEM;
>> -- 
>> 2.1.4
>>
>
>
>




More information about the samba-technical mailing list