[PATCH v4 6/7] Replace alloc+strcpy+strcat with asprintf() & XNFasprintf() calls

Alan Coopersmith alan.coopersmith at oracle.com
Sun Dec 5 09:51:08 PST 2010


walter harms wrote:
>> diff --git a/hw/xfree86/common/xf86Option.c b/hw/xfree86/common/xf86Option.c
>> index af39b2b..d49aa31 100644
>> --- a/hw/xfree86/common/xf86Option.c
>> +++ b/hw/xfree86/common/xf86Option.c
>> @@ -638,13 +638,10 @@ ParseOptionValue(int scrnIndex, pointer options, OptionInfoPtr p,
>>  	    newn = n + 2;
>>  	} else {
>>  	    free(n);
>> -	    n = malloc(strlen(p->name) + 2 + 1);
>> -	    if (!n) {
>> +	    if (asprintf(&n, "No%s", p->name) == -1) {
>>  		p->found = FALSE;
>>  		return FALSE;
>>  	    }
>> -	    strcpy(n, "No");
>> -	    strcat(n, p->name);
>>  	    newn = n;
>>  	}
> 
> Is n needed here at all ? it seems that asprintf(&newn, "No%s", p->name) would work also.

Further down in that function it cleans up with free(n);
which either clears the previous value if we didn't go
through this else clause or frees the value allocated
here - not using n here means making that cleanup more
complicated.


>> diff --git a/hw/xfree86/common/xf86ShowOpts.c b/hw/xfree86/common/xf86ShowOpts.c
>> index ce86090..c0fa80a 100644
>> --- a/hw/xfree86/common/xf86ShowOpts.c
>> +++ b/hw/xfree86/common/xf86ShowOpts.c
>> @@ -97,11 +97,8 @@ void DoShowOptions (void) {
>>  				);
>>  				continue;                                                       
>>  			}
>> -			pSymbol = malloc(
>> -				strlen(xf86DriverList[i]->driverName) + strlen("ModuleData") + 1
>> -			);
>> -			strcpy (pSymbol, xf86DriverList[i]->driverName);
>> -			strcat (pSymbol, "ModuleData");
>> +			XNFasprintf(&pSymbol, "%sModuleData",
>> +				    xf86DriverList[i]->driverName);
> 
> 
> every code before checks the return value of asprintf but not here ?

We don't need to know the length, and error is not an option from the XNF*
variants - they either succeed or cause the server to abort.   I used the
XNF* here since if the malloc failed, previously the server exited, just
going through the sigsegv handler instead of a cleaner AbortServer().
(NF is "No Fail")

-- 
	-Alan Coopersmith-        alan.coopersmith at oracle.com
	 Oracle Solaris Platform Engineering: X Window System



More information about the xorg-devel mailing list