On 31/08/18 16:50, Eric Blake wrote:
On 08/31/2018 10:36 AM, Liam Merwick wrote:
On 30/08/2018 17:18, Eric Blake wrote:
On 08/30/2018 10:47 AM, Liam Merwick wrote:
Incorrect checking of flags could result in uninitialized
file descriptor being used.
Looking at it again, the very minor optimisation of converting the
2nd 'if' to an 'else if' has the useful side-effect of appeasing the
static analysis tool.
I never figured out what the tool precisely thought was wrong in the
first place. Can you paste the output of the tool to see exactly what
it analyzed as the potential flaw? Perhaps the analyzer was trying to
see what would happen if a caller submitting the fourth value (3 on
systems where O_RDONLY is 0, 0 on systems where O_RDONLY is 1) and the
code not behaving in that setup, even though we know that all callers
only submit the three valid values and never the fourth invalid
value? Or maybe it's a weakness where we have made dependent
assumptions but in independent branches, in such a way that we know it
will never fail but the analyzer doesn't?
The specific error it reported was
Error: File Invalid
File Descriptor not Initialized [file-desc-not-init]:
The value <unknown> is not initialized as a file descriptor.
at line 91 of io/channel-command.c in function
'qio_channel_command_new_spawn'.
resource not initialized when flags != 0 at line 62
and flags != 1 at line 65
and stdinnull is false at line 69
and stdoutnull is false at line 69.
I've been staring at the code and can see no reason why it isn't a false
positive (and I'll let the tool authors know)
Regards,
Liam
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -61,8 +61,7 @@ qio_channel_command_new_spawn(const char *const
argv[],
if (flags == O_RDONLY) {
stdinnull = true;
- }
- if (flags == O_WRONLY) {
+ } else if (flags == O_WRONLY) {
But this sort of change is acceptable, especially if it does shut up
the analyzer, and done with a commit message mentioning that it is not
a semantic change but does make it easier to use the static checker to
find real problems by getting rid of noise.
stdoutnull = true;
}
Regards,
Liam
