diff mbox

Questionable code in gcov-io.c

Message ID 639591ca-c3ee-2c96-e185-78e04b869776@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Oct. 12, 2016, 2:47 p.m. UTC 
On 10/12/16 09:43, Marek Polacek wrote:

>> ITYM lines 197 -> 203.  I.e. remove the entire if that;s inside the 'mode ==
>> 0' branch and make line 203 unconditional.
>
> Yes, sorry for sloppy wording.  I'm testing a patch.

Here's the patch I've tested (but not bootstrapped).  As the incoming mode is 
-1, 0 or +1 we can make things considerably simpler.

nathan

Comments

Marek Polacek Oct. 12, 2016, 2:49 p.m. UTC | #1 
On Wed, Oct 12, 2016 at 10:47:07AM -0400, Nathan Sidwell wrote:
> On 10/12/16 09:43, Marek Polacek wrote:
> 
> > > ITYM lines 197 -> 203.  I.e. remove the entire if that;s inside the 'mode ==
> > > 0' branch and make line 203 unconditional.
> > 
> > Yes, sorry for sloppy wording.  I'm testing a patch.
> 
> Here's the patch I've tested (but not bootstrapped).  As the incoming mode
> is -1, 0 or +1 we can make things considerably simpler.

Great, thanks.

	Marek
Andreas Schwab Oct. 12, 2016, 3:04 p.m. UTC | #2 
On Okt 12 2016, Nathan Sidwell <nathan@acm.org> wrote:

> @@ -182,9 +176,7 @@ gcov_open (const char *name, int mode)
>        return 0;
>      }
>  
> -  if (mode > 0)
> -    gcov_var.mode = 1;
> -  else if (mode == 0)
> +  if (mode == 0)
>      {
>        struct stat st;
>  
> @@ -194,21 +186,20 @@ gcov_open (const char *name, int mode)
>  	  gcov_var.file = 0;
>  	  return 0;
>  	}
> -      if (st.st_size != 0)
> -	gcov_var.mode = 1;
> -      else
> -	gcov_var.mode = mode * 2 + 1;
> +      gcov_var.mode = 1;

Do we still need to call fstat?  I don't think it can ever fail here.

Andreas.
Nathan Sidwell Oct. 12, 2016, 3:12 p.m. UTC | #3 
On 10/12/16 11:04, Andreas Schwab wrote:

> Do we still need to call fstat?  I don't think it can ever fail here.

You appear to be correct.

nathan
diff mbox

Patch

2016-10-12  Nathan Sidwell  <nathan@acm.org>

	* gcov-io.c (gcov_open): Fix documentation.  Simplify setting
	gcov_var.mode.

Index: gcov-io.c
===================================================================
--- gcov-io.c	(revision 241027)
+++ gcov-io.c	(working copy)
@@ -115,10 +115,9 @@  static inline gcov_unsigned_t from_file
    opened. If MODE is >= 0 an existing file will be opened, if
    possible, and if MODE is <= 0, a new file will be created. Use
    MODE=0 to attempt to reopen an existing file and then fall back on
-   creating a new one.  If MODE < 0, the file will be opened in
+   creating a new one.  If MODE > 0, the file will be opened in
    read-only mode.  Otherwise it will be opened for modification.
-   Return zero on failure, >0 on opening an existing file and <0 on
-   creating a new one.  */
+   Return zero on failure, non-zero on success.  */
 
 GCOV_LINKAGE int
 #if IN_LIBGCOV
@@ -156,17 +155,12 @@  gcov_open (const char *name, int mode)
       /* pass mode (ignored) for compatibility */
       fd = open (name, O_RDONLY, S_IRUSR | S_IWUSR);
     }
-  else if (mode < 0)
+  else
      {
        /* Write mode - acquire a write-lock.  */
        s_flock.l_type = F_WRLCK;
-      fd = open (name, O_RDWR | O_CREAT | O_TRUNC, 0666);
-    }
-  else /* mode == 0 */
-    {
-      /* Read-Write mode - acquire a write-lock.  */
-      s_flock.l_type = F_WRLCK;
-      fd = open (name, O_RDWR | O_CREAT, 0666);
+       /* Truncate if force new mode.  */
+       fd = open (name, O_RDWR | O_CREAT | (mode < 0 ? O_TRUNC : 0), 0666);
     }
   if (fd < 0)
     return 0;
@@ -182,9 +176,7 @@  gcov_open (const char *name, int mode)
       return 0;
     }
 
-  if (mode > 0)
-    gcov_var.mode = 1;
-  else if (mode == 0)
+  if (mode == 0)
     {
       struct stat st;
 
@@ -194,21 +186,20 @@  gcov_open (const char *name, int mode)
 	  gcov_var.file = 0;
 	  return 0;
 	}
-      if (st.st_size != 0)
-	gcov_var.mode = 1;
-      else
-	gcov_var.mode = mode * 2 + 1;
+      gcov_var.mode = 1;
     }
   else
-    gcov_var.mode = mode * 2 + 1;
+    gcov_var.mode = mode;
 #else
   if (mode >= 0)
+    /* Open an existing file.  */
     gcov_var.file = fopen (name, (mode > 0) ? "rb" : "r+b");
 
   if (gcov_var.file)
     gcov_var.mode = 1;
   else if (mode <= 0)
     {
+      /* Create a new file.  */
       gcov_var.file = fopen (name, "w+b");
       if (gcov_var.file)
 	gcov_var.mode = mode * 2 + 1;