[PATCH] parser/scan.c: use strdup

Matt Turner mattst88 at gmail.com
Sat Jun 19 11:01:24 PDT 2010


On Wed, Jun 16, 2010 at 4:33 PM, Alan Coopersmith
<alan.coopersmith at oracle.com> wrote:
> Matt Turner wrote:
>> Found using
>> ---
>> @@
>> expression E;
>> identifier i;
>> identifier address;
>> @@
>>
>> - i = malloc(strlen(E) + 1);
>> - strcpy(address, E);
>> + i = strdup(E);
>> ---
>
> Would it be possible to similarly find the other common variant of
>        i = malloc(strlen(E) + 1);
>        if (i == NULL)
>            /* return 0, FatalError, panic, bail, maybe cleanup first */
>        strcpy(address, E);
> ?

I'm no coccinelle expert, but I came up with

---
@@
expression E;
identifier i;
identifier address;
statement S;
@@

- i = malloc(strlen(E) + 1);
- if (i == NULL) { ... S ... }
- strcpy(address, E);
+ i = strdup(E);
---

and it caught a few more. So new patch should be in your inbox.
Interestingly though, I just happened to come across another instance
in hw/xfree86/parser/scan.c around line 451 that neither this semantic
patch nor the original caught.

Matt

> Either way, it's much easier to verify strdup is correct than a
> malloc+strcpy pair, so I like it.
>
>> Signed-off-by: Matt Turner <mattst88 at gmail.com>
>> ---
>>  hw/xfree86/parser/scan.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
>> index 5312143..9f63570 100644
>> --- a/hw/xfree86/parser/scan.c
>> +++ b/hw/xfree86/parser/scan.c
>> @@ -1088,8 +1088,7 @@ void
>>  xf86setSection (char *section)
>>  {
>>       free(configSection);
>> -     configSection = malloc(strlen (section) + 1);
>> -     strcpy (configSection, section);
>> +     configSection = strdup(section);
>>  }
>>
>>  /*
>
> Reviewed-by: Alan Coopersmith <alan.coopersmith at oracle.com>
>
> --
>        -Alan Coopersmith-        alan.coopersmith at oracle.com
>         Oracle Solaris Platform Engineering: X Window System
>
>


More information about the xorg-devel mailing list