Re: dd PATCH: add conv=direct

[Paul Eggert <eggert@CS.UCLA.EDU>]

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.

Index: src/dd.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/dd.c,v
retrieving revision 1.157
diff -p -u -r1.157 dd.c
--- src/dd.c	8 Apr 2004 15:25:39 -0000	1.157
+++ src/dd.c	8 Apr 2004 19:17:02 -0000
@@ -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,9 +1022,9 @@ 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);
       if (old_flags < 0
-	  || (new_flags != old_flags && fcntl (fd, F_SETFL, new_flags) == -1))
+	  || (old_flags != (old_flags | add_flags)
+	      && fcntl (fd, F_SETFL, old_flags | add_flags) == -1))
 	error (EXIT_FAILURE, errno, _("setting flags for %s"), quote (name));
     }
 }