Code review comment for lp:~christian-roeller/hipl/whitelisting

Revision history for this message
Christof Mroz (christof-mroz) wrote :

On Tue, 13 Sep 2011 21:09:23 +0200, Christian Röller
<email address hidden> wrote:

> But I think to comment this in the function is enough, because an
> assertion would mean additional computations, which are not necessary
> because the assertion would never fire.

That's the point of using an assertion versus a regular check: if
performance matters (in release builds etc.), you should be able to
undef the HIP_ASSERT macro (and thus don't compile the checks) without
breaking anything. In this sense, assertions don't cost anything.

> I tried this and the compiler doesn't complain. I could choose every
> size vor char label[].
> Are you sure, that this will be checked from the compiler? I think
> this would only be the case, if I will really declare label as
> static. Because only these variables are known to the compiler during
> the compilephase. For label[IF_NAMESIZE] will first memory allocated
> during the execution. Maybe this is the reason why the compiler
> doesn't complain. Although I would say this must be verifiable during
> the compilephase. But I am not sure if the compiler will do it.
> But maybe I am totaly wrong here. What would you say?

Just tried it myself, and you're right: no sanity checks when passing
buffers around. Not even bounds checking works for parameter arrays;
it's as if the size info is discarded altogether. Which can't be true
because the compiler needs to carry it around for multidimensional
arrays, where all but one dimension must be specified. Meh, never
trust compilers...

Why did I think it would work in the first place? Because gcc actually
does (static, i.e. no-cost) bounds-checking e.g. for stack-allocated
arrays if told so. But Parameters are a whole other affair, it seems.

That said: feel free to omit the size, though I'd say it improves
documentation.

« Back to merge proposal