Fixed two Coverity defects

[dimkr]

1. strlen() on uninitialized string
2. Ignoring the return value of chmod()

[Sonophoto]

So what happens in the very odd case where we can get a file handle but can't immediately run a system call on it?

I would however cast the return value (void) as a reminder we didn't need the return value.

[dimkr]

So what happens in the very odd case where we can get a file handle but can't immediately run a system call on it?

Theoretically, fopen() and chmod() can be replaced with a combination of open(), chmod() and fdopen(), but I can't think of a good reason to do so.

[MMI]

Or you could fchmod(fileno(fp), ...)

[dimkr]

Unless you want "run a system call on it" to happen before "get a file handle", for some reason

[MMI]

Admittedly, I've paid zero attention to the larger context but...

fopen(..., "w") creates a new file... it's possible that the file doesn't exist prior to the fopen

I'm guessing that the intent of this snippet is to create a file and make it read/write only for the creator (0600)... what the author is protecting against, I don't know.

[MMI]

oh, I see your point -- meh, not sure that deferring the creation of the FILE * saves all that much

[Sonophoto]

so FYI, all of this is here because of an old bug in redis where the history file (passwords et. al.) were being saved world readable if the user did not have a sane umask set.

So the fix, if one is needed, should be in the context of solving the actual problem more correctly - not just quieting Coverity.

Personally I would just cast the return on chmod() (void).

[Sonophoto]

This was the fix for the history file saving issue:

c894b9e

[MMI]

As I commented on the referenced commit, why have the chmod at all?

[MMI]

No idea how I became a reviewer but I think these changes are sane.

[rain-1]

This implements the same change as #151

also adds a (void) cast to chmod, I can understand the value of coverity but I don't see this as a valuable addition at the moment.

This patch does not need to be merged.

[dimkr]

I can understand the value of coverity but I don't see this as a valuable addition at the moment.

Compiler warnings aside (unused return value, etc'), the value I see is getting rid of one (useless) Coverity error in every project that uses linenoise upstream as a git submodule.

[Sonophoto]

@rain-1 All of this has been discussed to death, this is done differently than #151, as has been discussed at length as well. @antirez has never chosen to accept these changes but they have been left open if he changes his mind at some future time.

[rain-1]

Merged rain-1@6d759a8