On Wed, Jan 28, 2015 at 04:06:30PM +0000, Dimitri John Ledkov wrote:
> On 28 January 2015 at 15:47, Martin Pitt <martin.p...@ubuntu.com> wrote:
> > Hey Zbyszek,
> >
> > Zbigniew Jędrzejewski-Szmek [2015-01-28 15:37 +0100]:
> >> > +static int handle_requests(int socket_fd) {
> >> > + Clients *first = NULL;
> >> > + usec_t last_activity = 0;
> >> > + int numdevices = 0, clear = 0;
> >> > + double percent = 100;
> >> > + _cleanup_fclose_ FILE *console = NULL;
> >> NULL not necessary (and you don't have it in similar circumstances above
> >> ;))
> >
> > How is that not necessary? Just because the very next line initializes
> > it, and there is no exit path before?
Yes.
> > Because in the general case it
> > certainly is necessary, otherwise the _cleanup_ handler will try to
> > free/close/clean up a random pointer value and crash.
Of course, but I think that in this case it is pretty obvious when
that the next line overwrites the NULL.
> > So IMHO it's a good defensive habit to always init _cleanup_* vars
> > with NULL. In the future someone might put some code with "return"
> > before the fopen() below, and then get a crash.
They can screw up in this and a thousand other ways. If there's a bunch
of other code in between, I agree that adding a defensive initialization
is good. In a simple case like this, I'm fine with skipping it.
Please note though, that my initial complaint was that there's an
inconsistency in the patch: one place had it, one didn't.
> > Or is there some gcc magic that I missed? (I thought only global
> > variables were guaranteed to be NULL, not stack vars).
No.
>
> Well, during systemd build there are some -Wmaybe-uninitialized
> warnings.... in most cases it is code similar to the above case or
> like checking r || check uninitialised variable. Shall we fix all of
> them? In most cases it would be of limited gain, but at least it will
> be clear to see when -Wmaybe-uninitalized warnings catches a real
> thing.
gcc issues false positive warnings in two cases:
1. when the same conditional is used twice, and cannot change:
bool cond = ...;
int var;
if (cond)
var = ...;
...
if (cond)
use(var);
2. when there's a loop which must be executed at least one:
int var;
while(true) {
var = ...;
break;
}
use(var);
Adding workarounds is frowned upon, unless they are very simple. We
try not to have warnings in the -O0 compilation mode. There's just too
many false warnings at higher optimization levels to fix all.
> (or is my compiler old and has false positives?)
Hard to say, not knowing the version :)
gcc5 issues approx. 100000 new warnings.
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
