Re: [Gap-dev-discuss] Cynthiune : fix a string comparison

[Philippe Roussel]

Le 04/05/2012 08:30, Sebastian Reitenbach a écrit :
>  
> On Thursday, May 3, 2012 19:56 CEST, Philippe Roussel <address@hidden> wrote: 
>  
>> Hi,
>>
>> The code looks stupid :
> 
> Right, that seems to be stupid, but I was wondering whether stcmp 
> might be prone to buffer overflows, but after thinking a bit about it, 
> I don't think so. But maybe using strncmp to be on the safe side
> might still be better?
> 
> Haven't tested, but maybe:
> if !(strncmp(commentTag, ID3_FRAME_GENRE, 
> strlen(commentTag)>strlen(ID3_FRAME_GENRE)?strlen(ID3_FRAME_GENRE):strlen(commentTag)))
> 
> Or might be overkill?

If you want to use strncmp I think !strncmp(commentTag, ID3_FRAME_GENRE,
strlen(ID3_FRAME_GENRE)) is enough.

But after looking closely at the code, even the original code is correct
as this method is only called with only static strings from the library

[self _readComment: ID3_FRAME_TITLE fromTag: id3Tag]
[self _readComment: ID3_FRAME_ARTIST fromTag: id3Tag]
[self _readComment: ID3_FRAME_GENRE fromTag: id3Tag]

etc so the pointer comparison is actually correct !

Anyway, using strcmp is nicer and safer (someone could use the method
with another string).

Thanks,
Philippe

> Sebastian
> 
>>
>> diff -u -r1.1.1.1 ID3Tag.m
>> --- Bundles/ID3Tag/ID3Tag.m  25 Mar 2012 07:48:21 -0000      1.1.1.1
>> +++ Bundles/ID3Tag/ID3Tag.m  3 May 2012 17:55:44 -0000
>> @@ -91,7 +91,7 @@
>>            string = id3_field_getstrings (field, 0);
>>            if (string)
>>              {
>> -              if (commentTag == ID3_FRAME_GENRE)
>> +              if (!strcmp(commentTag, ID3_FRAME_GENRE))
>>                  string = id3_genre_name (string);
>>                comment = [NSString stringWithUCS4String: string];
>>              }