[Mesa-dev] [RFC PATCH 0/4] proposed scan-build cleanups

[Giuseppe Bilotta]

Hello,

On Sat, Apr 22, 2017 at 11:46 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> Definitely split the changes up along a combination of where they are in the
> source and what types of errors they are. I'd say the split you've chosen
> looks reasonable to me.

Excellent, thanks.

>> 1. the addition of a check to malloc return value; this was rather
>> trivial per se, but for example while going to through the source code I
>> noticed that for _reallocs_ the ‘safe’ approach is generally not
>> followed (store the old pointer before the realloc so that if the
>> realloc fails we don't get a memory leak), and I was wondering if a
>> patchset to fortify `realloc()` would be worth it (especially
>> considering that there's mostly small reallocs and if they fail the
>> situation is catastrophic enough that the memory leak would be the last
>> of the problems).
>
> I'm generally in favor of fixing our use of realloc. I occasionally do that
> in code that I'm working on anyway.

Good, then I'll try to spin a patchset just for the realloc safety.

> Well, you've more or less already explained the difference: the asset +
> return will actually return something possibly meaningful in optimized
> builds, whereas with unreachable, almost anything might happen: it's
> basically at the whim of the compiler.
>
> For example: In the case of large switch statements that end up being
> lowered into jump tables, the unreachable will probably end up jumping to an
> arbitrary address and likely crash, while for small switch statements, an
> arbitrary non-unreachable path is likely to be executed.
>
> For the most part, I think the use of assert + return is there because the
> code was written at a time when unreachable builtins simply didn't exist.
> I'm generally in favor of using unreachable; it contains an assert, so
> testing on builds with assertions enabled will catch them.
>
> I have one remark on patch 3. Patches 1,2,4 are:
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Thanks for the clarification and the review. I'll proceed with the
unreachable() then if I find more situations such as this, and then
individual cases where it might be appropriate can be weeded out in
review.

Best regards,

-- 
Giuseppe "Oblomov" Bilotta