[asterisk-dev] [Code Review] 4530: clang compiler warning: -Wgnu-variable-sized-type-not-at-end

[rmudgett]

> On March 26, 2015, 8:11 p.m., Matt Jordan wrote:
> > /branches/13/main/stdtime/localtime.c, lines 357-361
> > <https://reviewboard.asterisk.org/r/4530/diff/1/?file=72936#file72936line357>
> >
> >     Just curious, what was the compiler warning on this one?
> 
> Diederik de Groot wrote:
>     There can only be one variable size struct element and it has to be at the last position. Having them anywhere in the struct is a gcc extension.
> 
> rmudgett wrote:
>     This is not a valid fix!!  The buf.name field is supposed to be the extra space that the buf.iev.name field represents.
> 
> rmudgett wrote:
>     I'd suggest using ast_alloca() or alloca() to create space for buf instead.
> 
> Diederik de Groot wrote:
>     I did not interpret this correctly i guess. Doesn't this mean a union should have been used instead ?
>     Can you give me an example of how you would restructure this using alloca.

Unions have all members overlapping each other.  That is not what is needed here.  The struct inotify_event has a last member named name and the locally defined struct for buf is reserving space for buf.iev.name with the buf.name array.

Since clang is complaining about this we need to change the definition of buf and the usage of it in the rest of the function.

struct inotify_event *buf;
size_t real_sizeof_buf = sizeof(*buf) + FILENAME_MAX + 1;

buf = ast_alloca(real_sizeof_buf);

Where used:
sizeof(buf) becomes real_sizeof_buf
sizeof(buf.iev) becomes sizeof(*buf)
buf.iev.wd becomes buf->wd

Since buf is not a good name because it is now misleading, renaming buf to iev would be better.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4530/#review14875
-----------------------------------------------------------


On March 26, 2015, 1:12 p.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4530/
> -----------------------------------------------------------
> 
> (Updated March 26, 2015, 1:12 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24917
>     https://issues.asterisk.org/jira/browse/ASTERISK-24917
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> clang's static analyzer will throw quite a number warnings / errors during compilation, some of which can be very helpfull in finding corner-case bugs\nclang compiler warning:-Wgnu-variable-sized-type-not-at-end
> 
> 
> Diffs
> -----
> 
>   /branches/13/main/stdtime/localtime.c 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4530/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150327/eb10f303/attachment.html>