Messages in this thread Patch in this message | | | Subject | Re: dd PATCH: add conv=direct | From | Jim Meyering <> | Date | Thu, 08 Apr 2004 23:34:25 +0200 |
| |
Paul Eggert <eggert@CS.UCLA.EDU> wrote: > Jim Meyering <jim@meyering.net> writes: > >> 2004-04-08 Jim Meyering <jim@meyering.net> >> >> * 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 <eggert@twinsun.com> > > * 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)); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |