Paul Eggert <[EMAIL PROTECTED]> wrote: > Jim Meyering <[EMAIL PROTECTED]> writes: > >> 2004-04-08 Jim Meyering <[EMAIL PROTECTED]> >> >> * src/dd.c (set_fd_flags): Don't OR in -1 when fcntl fails. > > Doesn't that fix generate worse code in the usual case, since it > causes two conditional branches instead of one? > > How about this further patch? It relies on common subexpression > elimination, but that's common these days. > > 2004-04-08 Paul Eggert <[EMAIL PROTECTED]> > > * src/dd.c (set_fd_flags): Don't test old_flags < 0 twice.
I see your point. Thanks for the comment fix. I've gone ahead and reverted my change, since I think your original code is a little more readable than the more-efficient-when-fcntl-fails code that you're proposing. (set_fd_flags): Undo part of today's change: it's a little cleaner -- and more efficient in the common case -- to go ahead and OR in the -1 when fcntl fails. Index: src/dd.c =================================================================== RCS file: /fetish/cu/src/dd.c,v retrieving revision 1.158 retrieving revision 1.159 diff -u -p -u -r1.158 -r1.159 --- a/src/dd.c 8 Apr 2004 21:26:28 -0000 1.158 +++ b/src/dd.c 8 Apr 2004 21:30:18 -0000 1.159 @@ -1014,7 +1014,7 @@ copy_with_unblock (char const *buf, size } /* Set the file descriptor flags for FD that correspond to the nonzero bits - in FLAGS. The file's name is NAME. */ + in ADD_FLAGS. The file's name is NAME. */ static void set_fd_flags (int fd, int add_flags, char const *name) @@ -1022,7 +1022,7 @@ set_fd_flags (int fd, int add_flags, cha if (add_flags) { int old_flags = fcntl (fd, F_GETFL); - int new_flags = old_flags < 0 ? add_flags : (old_flags | add_flags); + int new_flags = old_flags | add_flags; if (old_flags < 0 || (new_flags != old_flags && fcntl (fd, F_SETFL, new_flags) == -1)) error (EXIT_FAILURE, errno, _("setting flags for %s"), quote (name)); _______________________________________________ Bug-coreutils mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/bug-coreutils