Patch for x86_32: Fix bug 14195

Andreas Jaeger aj@suse.com
Wed Aug 15 19:04:00 GMT 2012


On 08/15/2012 11:33 AM, Dmitrieva Liubov wrote:
> Thanks for the review.
>
>> Missing copyright year update and merge.
> Added.
>
>> Please add a comment saying "/* Regression test for BZ #14195.  */
> Done.
>
>> The code you are adding should be in a function labelled after
>> the BZ# that you are fixing e.g. bz14195();.
> Fixed.
>
> I've reattached the patch and Change Log is the same.
>
> ChangeLog:
>
> 2012-08-15  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>
>           [BZ #14195]
>           * sysdeps/i386/i686/multiarch/strcmp-sssse3.S: Fix
>           segmentation fault for a case of two empty input strings.
>           * string/test-strncasecnp.c: Update.
>           Add a test-case for two empty input strings and N > 0

The changelog should be IMO:

	* string/test-strncasecmp.c (check1): Renamed to...
	(bz12205): ...this.
	(bz14195): Add new testcase.
	(test_main): Call new testcase, adapt for renamed function.

> --
> Liubov Dmitrieva
>
> 2012/8/14 Carlos O'Donell <carlos_odonell@mentor.com>:
>> On 8/14/2012 7:15 AM, Dmitrieva Liubov wrote:
>>> Ok, fixed version is attached.
>>
>> Thanks for working through the changes.
>>
>> It would be useful if you provide a summary of the changes since the
>> last version, that way the reviewers can see that you either accepted
>> or refuted all of the comments.
>>
>>> ChangeLog:
>>>
>>> 2012-08-14  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>>>
>>>           [BZ #14195]
>>>           * sysdeps/i386/i686/multiarch/strcmp-sssse3.S: Fix
>>>           segmentation fault for a case of two empty input strings.
>>>           * string/test-strncasecnp.c: Update.
>>>           Add a test-case for two empty input strings and N > 0
>>
>> Please pay more careful attention to the reviewer comments. There
>> are some things which you did not fix and they are important.
>>
>>> strncasecmp_fix.patch
>>>
>>>
>>> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
>>> index 6c17530..1b90e06 100644
>>> --- a/string/test-strncasecmp.c
>>> +++ b/string/test-strncasecmp.c
>>
>> Missing copyright year update and merge.

Merge means: replace 2008, 2010, 2012 with
2008-2012

I'm committing the change now for you since AFAIK you don't have git access,

Thanks,
Andreas

>>
>>> @@ -270,6 +270,14 @@ check1 (void)
>>>       check_result (impl, s1, s2, n, exp_result);
>>>   }
>>>
>>
>> Please add a comment saying "/* Regression test for BZ #14195.  */
>>
>>> +static void
>>> +bz12205 (void)
>>
>> This is wrong. Please review what I said in my original email.
>> BZ #12205 is the BZ # for the regression test that was added
>> as check1();.
>>
>> The code you are adding should be in a function labelled after
>> the BZ# that you are fixing e.g. bz14195();.
>>
>>> +{
>>> +  const char *empty_string  = "";
>>> +  FOR_EACH_IMPL (impl, 0)
>>> +    check_result (impl, empty_string, "", 5, 0);
>>> +}
>>> +
>>>   int
>>>   test_main (void)
>>>   {
>>> @@ -278,6 +286,7 @@ test_main (void)
>>>     test_init ();
>>>
>>>     check1 ();
>>> +  bz12205 ();
>>>
>>>     printf ("%23s", "");
>>>     FOR_EACH_IMPL (impl, 0)
>>> diff --git a/sysdeps/i386/i686/multiarch/strcmp-ssse3.S b/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
>>> old mode 100644
>>> new mode 100755
>>> index 5e6321e..9735ad0
>>> --- a/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
>>> +++ b/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
>>> @@ -2445,7 +2445,7 @@ L(less16bytes_sncmp):
>>>   # endif
>>>        jne     L(neq_sncmp)
>>>        test    %cl, %cl
>>> -     je      L(eq)
>>> +     je      L(eq_sncmp)
>>>
>>>        cmp     $1, REM
>>>        je      L(eq_sncmp)
>>
>> Please fix the copyright year, function name, comment, and repost.
>>
>> You're doing a good job, but please pay more careful attention to the reviewer comments.
>>
>> Cheers,
>> Carlos.
>> --
>> Carlos O'Donell
>> Mentor Graphics / CodeSourcery
>> carlos_odonell@mentor.com
>> carlos@codesourcery.com
>> +1 (613) 963 1026


-- 
  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126



More information about the Libc-alpha mailing list