bugGuile - Bugs: bug #24009, does not check for short write()s

 
 

bug #24009: does not check for short write()s

Submitter:  Martin Pitt <martinpitt>
Submitted:  Wed 06 Aug 2008 11:51:02 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 30 Nov 2008 06:10:06 PM UTC, comment #5: 

Hello!

This is now fixed:

  master -> http://git.savannah.gnu.org/gitweb/?p=guile.git;a=commit;h=634aa8de8f8facc285867100c6c0a54c4ea5e6d4
  1.8 -> http://git.savannah.gnu.org/gitweb/?p=guile.git;a=commit;h=b12a5fd7f645067143523094194db0154a890539

The version in `master' is more robust since it uses `full_write ()' from Gnulib, which tries hard to actually write the given bytes; the fix in 1.8 does the minimum to make Glibc happy with `_FORTIFY_SOURCE=2'.

Most of the cases reported by `_FORTIFY_SOURCE' are when writing a single byte to the signal pipe or the sleep FD.  In these situations, it would be hard to handle a write/read error meaningfully, and it can't be very harmful if one of these bytes is lost.

One last thing: the Flex-generated file `c-tokenize.c' does not check the return value of `fwrite(3)', but there's nothing we can do here.  Another good reason to not make `-Werror' the default?

Thanks,
Ludo'.

Ludovic Courtès <civodul>
Group administrator
Fri 31 Oct 2008 02:45:56 AM UTC, comment #4: 

I didn't have anything real, I was just checking that the return value didn't match the correct length and returning.  I suppose asserts could be used instead.

Adam Schreiber <sadam>
Thu 30 Oct 2008 10:44:17 PM UTC, comment #3: 

Thanks Adam for the extra info, I'll try to work on this soon.  In case you already have a patch that you're using, please could you attach it to this bug?

- Neil

Anonymous
Wed 29 Oct 2008 04:58:00 PM UTC, comment #2: 

This bug breaks building GNOME 2.26 in jhbuild because it treats warnings like errors.

Adam Schreiber <sadam>
Wed 06 Aug 2008 08:08:29 PM UTC, comment #1: 

Thanks for noticing and reporting this.  I think you are right - at least for the one in async.c (I haven't checked any others yet), so we'll hopefully get to fixing this soon.

     Neil

Neil Jerram <ossau>
Group administrator
Wed 06 Aug 2008 11:51:02 AM UTC, original submission:  

When building guile under current ubuntu (gcc-4.3, with FORTIFY_SOURCE=2), it detects a lot of cases where write() is called without checking the return value:

/bin/bash ../libtool --tag=CC   --mode=compile i486-linux-gnu-gcc -DHAVE_CONFIG_H  -I.. -I.. -I..   -pthread -O2 -g -Wall -Wmissing-prototypes -Werror -MT libguile_la-async.lo -MD -MP -MF .deps/libguile_la-async.Tpo -c -o libguile_la-async.lo `test -f 'async.c' || echo './'`async.c
 i486-linux-gnu-gcc -DHAVE_CONFIG_H -I.. -I.. -I.. -pthread -O2 -g -Wall -Wmissing-prototypes -Werror -MT libguile_la-async.lo -MD -MP -MF .deps/libguile_la-async.Tpo -c async.c  -fPIC -DPIC -o .libs/libguile_la-async.o
cc1: warnings being treated as errors
async.c: In function ‘scm_i_queue_async_cell’:
async.c:250: error: ignoring return value of ‘write’, declared with attribute warn_unused_result

Some of those might actually be harmless and can be cast into (void), but in most cases a short write is a source of unexpected and hard to debug errors. Maybe consider using a full_write() wrapper (like in gnulib) which repeatedly writes until everything is sent?

Martin Pitt <martinpitt>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by civodul (Julian Graham)
  • -email is unavailable- added by civodul (Posted a comment)
  • -email is unavailable- added by sadam (Posted a comment)
  • -email is unavailable- added by ossau (Posted a comment)
  • -email is unavailable- added by martinpitt (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-11-30 civodul Carbon-Copy- Added -email is unavailable-
    2008-11-30 civodul Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code