[systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261

[Ronny Chevalier]

2014-11-17 18:31 GMT+01:00 Greg KH <gregkh at linuxfoundation.org>:
> On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
>> On 11/17/2014 10:39 PM, Greg KH wrote:
>> >On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
>> >>On 11/17/2014 10:26 PM, Greg KH wrote:
>> >>>On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
>> >>>>---
>> >>>>  src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
>> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>>
>> >>>>diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >>>>index e6dc84b..1fc792b 100644
>> >>>>--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >>>>+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >>>>@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
>> >>>>                  return -ENOMEM;
>> >>>>
>> >>>>          mkdir_parents_label(p, 0700);
>> >>>>-        mkfifo(p, 0600);
>> >>>>
>> >>>>+        (void)mkfifo(p, 0600);
>> >>>
>> >>>You really aren't "fixing" anything in these patches, just merely
>> >>>papering over the Coverity issues.  Which is fine, if you really want to
>> >>>do that, but don't think it's anything other than that...
>> >>
>> >>Yes my intention is to for coverity only Any way next line 'open' handling
>> >>the error case .
>> >
>> >I'm sorry, but I don't understand this sentance at all, can you rephrase
>> >it?
>> >
>>
>> Sorry let me rephrase it. This patch only for coverity . The next like of
>> mkfifo is open .
>>
>> (void)mkfifo(p, 0600);
>> fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
>> if (fd < 0)
>>         return -errno;
>>
>> and open is handling the failure.
>
> Then coverity should be fixed, don't paper over stupid bugs in tools for
> no reason.
I disagree.

Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here. In this case Susant is
documenting the fact that he does not care about the return value of
mkfifo because he thinks that it is already handled by open. In
another program one can just forgot to check the return value of
mkfifo and doing an open after, but maybe in this program checking the
return value of mkfifo is important.

>
> thanks,
>
> greg k-h
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel