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
