Questionable code in gcov-io.c

[Bernd Schmidt]

On 10/12/2016 02:10 PM, Marek Polacek wrote:
> While implementing a warning I noticed this in gcov-io.c:
>
>  187   else if (mode == 0)
>  188     {
>  189       struct stat st;
>  190
>  191       if (fstat (fd, &st) < 0)
>  192         {
>  193           fclose (gcov_var.file);
>  194           gcov_var.file = 0;
>  195           return 0;
>  196         }
>  197       if (st.st_size != 0)
>  198         gcov_var.mode = 1;
>  199       else
>  200         gcov_var.mode = mode * 2 + 1;
>  201     }
>  202   else
>  203     gcov_var.mode = mode * 2 + 1;
>
> It seems that lines 198 and 200 do the same thing, at line 200 we know that
> mode == 0, so we just assign 1.  Should we just remove the condition on line 197?

The intention seems to be that a negative gcov_var.mode means we're 
writing, so for a size zero file maybe that's the expected result. But 
of course none of the existing code is going to expect that.

There are more oddities here...

Before the quoted piece of code, we test for mode > 0, so in line 203 we 
know that mode is < 0. I don't really see what the "mode * 2 + 1" 
expression is supposed to do anyway, given that the caller could pass in 
any positive/negative number and expect the same results as for 1 and 
-1. I think line 203 should just use -1. There's one further occurrence 
of that expression which should probably be modified.

The function comment also seems to have an issue:

/* Open a gcov file. NAME is the name of the file to open and MODE
    indicates whether a new file should be created, or an existing 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
    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.  */

This suggests that with MODE < 0, we'll create a file in read-only mode, 
which is nonsensical. The code suggests that the comment should read " > 0".


Bernd