Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Frank Lanitz]

Merged #916 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#event-2705032318

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Matthew Brush]

> it's actually technically correct to say the value may not be initialized 
> because of those checks

Depending on whether they're actually enabled, of course.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538803747

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Colomban Wendling]

> 2. and if it did, the return_ifs can shortcut return and leave it 
> uninitialised.

@elextr got a point, it's actually technically correct to say the value may not 
be initialized because of those checks, and it seem impossible for a tool to 
infer that those checks will succeed, even if we know they will -- unless 
something else went terribly bad and we're fubar.

So although I initially blamed that on cppcheck, I guess it's fair and this PR 
makes sense.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538789763

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[LarsGit223]

> In any case, I'm fine with this PR if it shuts up cppcheck; this isn't 
> performance critical in anyway, and it doesn't affect readability.

I agree to this and vote for merging it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538766979

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Enrico Tröger]

I don't mind much.
If you think it's a false-positive in `cppcheck`, then you should report it (or 
comment on https://trac.cppcheck.net/ticket/9347, it looks close to our case).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538766795

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[elextr]

> If it did proper analysis (ie. look into callee) it would see it's passed to 
> memcpy and fully written.

1. "if" :)

2. and if it, did the return_ifs can shortcut return and leave it uninitialised.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538177110

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Matthew Brush]

> But or not I think setting a variable to a default is a good thing.

I haven't done a proper survey, but I'd wager if we did, we'd find hundreds if 
not thousands of uninitialized variables in Geany and GP's code.

> there is nothing to tell cppcheck that argument 2 is an out

If it did proper analysis (ie. look into callee) it would see it's passed to 
`memcpy` and fully written.

In any case, I'm fine with this PR if it shuts up `cppcheck`; this isn't 
performance critical in anyway, and it doesn't affect readability.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538164617

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Matthew Brush]

codebrainz approved this pull request.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#pullrequestreview-297200041

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[elextr]

It would appear to be a correct warning, there is nothing to tell cppcheck that 
argument 2 is an out only argument so it has to assume the value is used in the 
function and hence an uninitialised value is being passed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538110962

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Frank Lanitz]

But or not I think setting a variable to a default is a good thing.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-538054220

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Matthew Brush]

Maybe it's a bug in cppcheck?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-537971260

Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)

[Enrico Tröger]

Newer cppcheck versions (probably 1.89 which is in Debian Sid since a few days) 
complained about it and so the nightly builds broke.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/916#issuecomment-537828269