> 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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to