Re: [ECOS] sscanf() exception issue of C lib

Re: [ECOS] sscanf() exception issue of C lib

[Jonathan Larmour]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 1488 bytes --]

Xiaochen Zhou wrote:
> 
> Xiaochen Zhou дµÀ:
> 
>>Jonathan Larmour дµÀ:
>>  
>>
>>>Xiaochen Zhou wrote:
>>>  
>>>    
>>>
>>>>I try hard to check my code, but does not work. At last I browse the c
>>>>lib code, and have some doubts with the
>>>>language/c/libc/stdio/current/src/common/fflush.cxx
>>>>
[snip]
>>I write a test thread to call sscanf() again and again in a loop, and in
>>the same time, I hit the F5 to flush the web pages of my device very
>>quickly (httpd has higher priority and call stdio lib's fdopen() and
>>close()). The exception repeated in few minutes. So I'm sure it is a
>>ecos lib c bug. Add the lock, after several hours testing, the exception
>>is gone. But it is not the exact evidence. I will add some testing code
>>to prove it today.
>>
>>  
> 
> Proved!!!

Yes, that does appear to confirm it. Thanks for tracking this down. Now
that I've looked more closely at this code, it's obvious really that the
files table should have been protected - that's the whole point of having
a mutex for it, so it was just an oversight that it wasn't - that's lasted
nearly 9 years!

Let me know if there are any problems with the attached patch, which I'm
checking in. Thanks.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------      Opinions==mine

Re: [ECOS] sscanf() exception issue of C lib

[Jonathan Larmour]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

Jonathan Larmour wrote:
> 
> Let me know if there are any problems with the attached patch, which I'm
> checking in. Thanks.

Ahem. Now attached.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------      Opinions==mine

[-- Attachment #2: fflush.fix.patch --]
[-- Type: text/x-patch, Size: 2425 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/stdio/current/ChangeLog,v
retrieving revision 1.43
retrieving revision 1.44
diff -u -5 -p -r1.43 -r1.44
--- ChangeLog	21 Jul 2008 10:38:23 -0000	1.43
+++ ChangeLog	15 Jan 2009 03:33:52 -0000	1.44
@@ -1,5 +1,11 @@
+2009-01-15  Jonathan Larmour  <jifl@eCosCentric.com>
+
+	* src/common/fflush.cxx (cyg_libc_stdio_flush_all_but): Ensure the
+	files table can't change. Thanks to Xiaochen Zhou for the detective
+	work.
+
 2008-07-21  Guenter Ebermann  <guenter.ebermann@gmx.at>
 
 	* src/common/fclose.cxx (fclose): Replace config-dependent use of
 	delete with free(), as the memory had been allocated with malloc.
 
Index: src/common/fflush.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/stdio/current/src/common/fflush.cxx,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -5 -p -r1.7 -r1.8
--- src/common/fflush.cxx	15 Mar 2004 15:21:44 -0000	1.7
+++ src/common/fflush.cxx	15 Jan 2009 03:33:52 -0000	1.8
@@ -80,11 +80,15 @@ cyg_libc_stdio_flush_all_but( Cyg_StdioS
     do {
         loop_again = false;
 
         for (i=0; (i<FOPEN_MAX) && !err; i++) {
             if (files_flushed[i] == false) {
-                
+                // Don't let the files table change e.g. by closing the file.
+                if ( Cyg_libc_stdio_files::lock() ) {
+                    err = EINTR;
+                    break;
+                }
                 stream = Cyg_libc_stdio_files::get_file_stream(i);
                 
                 if ((stream == NULL) || (stream == not_this_stream)) {
                     // if it isn't a valid stream, set its entry in the
                     // list of files flushed since we don't want to
@@ -116,10 +120,14 @@ cyg_libc_stdio_flush_all_but( Cyg_StdioS
                             loop_again = true;
                             looped = true;
                         }
                     }
                 } // else
+                // We can unlock and relock every loop as we only care
+                // about flushing streams that were open prior to this
+                // call. Any new streams can be ignored.
+                Cyg_libc_stdio_files::unlock()
             } // if
         } // for
     } // do
     while(loop_again && !err);
     

Re: [ECOS] sscanf() exception issue of C lib

[Xiaochen Zhou]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 501 bytes --]



Jonathan Larmour дµÀ:
> Jonathan Larmour wrote:
>   
>> Let me know if there are any problems with the attached patch, which I'm
>> checking in. Thanks.
>>     
>
> Ahem. Now attached.
>
> Jifl
>   
+ // Don't let the files table change e.g. by closing the file.

+                if ( Cyg_libc_stdio_files::lock() ) {
+                    err = EINTR;
+                    break;
+                }

In most time, Cyg_libc_stdio_files::lock() return true, flush would not
work.

Xiaochen Zhou

Re: [ECOS] sscanf() exception issue of C lib

[Jonathan Larmour]

Xiaochen Zhou wrote:
> + // Don't let the files table change e.g. by closing the file.
> 
> +                if ( Cyg_libc_stdio_files::lock() ) {
> +                    err = EINTR;
> +                    break;
> +                }
> 
> In most time, Cyg_libc_stdio_files::lock() return true, flush would not
> work.

Doh. I even had it the other way round before, and then "corrected" it.
Silly thinko. Fixed in obvious way, thanks.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine