This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch for x86_32: Fix bug 14195


On 8/13/2012 5:25 AM, Dmitrieva Liubov wrote:
> Updated patch with a test-case is attached, please review it.
> 
> ChangeLog:
> 
> 2012-08-13  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
> 

BZ# in the wrong place.

http://sourceware.org/glibc/wiki/Contribution%20checklist#Properly_Formatted_GNU_ChangeLog

>          * sysdeps/i386/i686/multiarch/strcmp-sssse3.S: BZ #14195
>          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
> 
> --
> Liubov Dmitrieva
> Intel Corporation
> 
>>> >> Does the testsuite catch this?
>>> >> If it doesn't, could you please add a case which will prevent this from
>>> >> regressing in the future?
>>> >> Comparing two empty strings seems like a corner-case we should be testing.
>>> >> Cheers,
>>> >> Carlos.
> 2012/8/10 Roland McGrath <roland@hack.frob.com>:
>>> >> 2012-08-10  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>>> >>
>>> >>         * sysdeps/i386/i686/multiarch/strcmp-sssse3.S: Update
>>> >>         Fix bug 14195
>> >
>> > This log entry is insufficient, as well as not fitting conventions.
>> > The mention of a bug number is as "[BZ #14195]" alone (indented) on the
>> > first line of the entry.  The text of the entry should be properly
>> > punctuated, and it must say something meaningful about what changed.
> 
> strncasecmp_fix.patch
> 
> 
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6c17530..11f1084 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c

Missing copyright year update and merge.

> @@ -268,6 +268,9 @@ check1 (void)
>    exp_result = simple_strncasecmp (s1, s2, n);
>    FOR_EACH_IMPL (impl, 0)
>      check_result (impl, s1, s2, n, exp_result);
> +  s1 = "";
> +  FOR_EACH_IMPL (impl, 0)
> +    check_result (impl, s1, "", 5, 0);

Move this to a function called bz14195 and add a comment that
says it is a regression test for BZ #14195.

Call it after the call to check1.

If you feel like cleaning up a big I'd also accept a patch with:
~~~
diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
index 6c17530..420bff0 100644
--- a/string/test-strncasecmp.c
+++ b/string/test-strncasecmp.c
@@ -252,8 +252,9 @@ do_random_tests (void)
 }


+/* Regression test for BZ #12205.  */
 static void
-check1 (void)
+bz12205 (void)
 {
   static char cp [4096+16] __attribute__ ((aligned(4096)));
   static char gotrel[4096] __attribute__ ((aligned(4096)));
@@ -277,7 +278,8 @@ test_main (void)

   test_init ();

-  check1 ();
+  /* Specific regression test cases.  */
+  bz12205 ();

   printf ("%23s", "");
   FOR_EACH_IMPL (impl, 0)
~~~

>  }
>  
>  int
> 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)

OK with those changes.

Cheers,
Carlos.
-- 
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]