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

[Diederik de Groot]

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

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

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

[Matt Jordan]

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



/branches/13/main/stdtime/localtime.c


Just curious, what was the compiler warning on this one?


- Matt Jordan


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

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

[Diederik de Groot]

> On March 27, 2015, 2:11 a.m., Matt Jordan wrote:
> > /branches/13/main/stdtime/localtime.c, lines 357-361
> > 
> >
> > Just curious, what was the compiler warning on this one?

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.


- Diederik


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


On March 26, 2015, 7: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, 7: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

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

[Matt Jordan]

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

Ship it!


Ship It!

- Matt Jordan


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

Re: [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
> > 
> >
> > 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.

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


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

Re: [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
> > 
> >
> > 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.

I'd suggest using ast_alloca() or alloca() to create space for buf instead.


- 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

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

[Diederik de Groot]

> On March 27, 2015, 2:11 a.m., Matt Jordan wrote:
> > /branches/13/main/stdtime/localtime.c, lines 357-361
> > 
> >
> > 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.

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. 


- Diederik


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


On March 26, 2015, 7: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, 7: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

Re: [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
> > 
> >
> > 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

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

[Diederik de Groot]

> On March 27, 2015, 2:11 a.m., Matt Jordan wrote:
> > /branches/13/main/stdtime/localtime.c, lines 357-361
> > 
> >
> > 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.
> 
> rmudgett wrote:
> 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.

Thanks... Resolves the issue and even looks cleaner than the original :-)


- Diederik


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


On March 26, 2015, 7: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, 7: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

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

[Diederik de Groot]

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

(Updated March 27, 2015, 6:10 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 (updated)
-

  /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

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

[Diederik de Groot]

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

(Updated March 29, 2015, 8:52 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 433717


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

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

[Matt Jordan]

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


So, small problem with this change. When compiling with gcc 4.8.2, I get the 
following warning:

In function ‘read’,
inlined from ‘inotify_daemon’ at stdtime/localtime.c:380:6:
/usr/include/x86_64-linux-gnu/bits/unistd.h:42:2: error: call to 
‘__read_chk_warn’ declared with attribute warning: read called with bigger 
length than size of the destination buffer [-Werror]
  return __read_chk_warn (__fd, __buf, __nbytes, __bos0 (__buf));


Interestingly, the build agents didn't kick this back. But we'll need to find a 
way to get this fixed for gcc as well.

- Matt Jordan


On March 29, 2015, 8:52 p.m., Diederik de Groot wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4530/
> ---
> 
> (Updated March 29, 2015, 8:52 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

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

[Matt Jordan]

> On March 29, 2015, 9:11 p.m., Matt Jordan wrote:
> > So, small problem with this change. When compiling with gcc 4.8.2, I get 
> > the following warning:
> > 
> > In function ‘read’,
> > inlined from ‘inotify_daemon’ at stdtime/localtime.c:380:6:
> > /usr/include/x86_64-linux-gnu/bits/unistd.h:42:2: error: call to 
> > ‘__read_chk_warn’ declared with attribute warning: read called with bigger 
> > length than size of the destination buffer [-Werror]
> >   return __read_chk_warn (__fd, __buf, __nbytes, __bos0 (__buf));
> > 
> > 
> > Interestingly, the build agents didn't kick this back. But we'll need to 
> > find a way to get this fixed for gcc as well.

So, this is interesting. Looking at unistd.h:

extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
   size_t __buflen) __wur;
extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
  size_t __nbytes), read) __wur;
extern ssize_t __REDIRECT (__read_chk_warn,
   (int __fd, void *__buf, size_t __nbytes,
size_t __buflen), __read_chk)
 __wur __warnattr ("read called with bigger length than size of "
   "the destination buffer");

__fortify_function __wur ssize_t
read (int __fd, void *__buf, size_t __nbytes)
{
  if (__bos0 (__buf) != (size_t) -1)
{
  if (!__builtin_constant_p (__nbytes))
return __read_chk (__fd, __buf, __nbytes, __bos0 (__buf));

  if (__nbytes > __bos0 (__buf))
return __read_chk_warn (__fd, __buf, __nbytes, __bos0 (__buf));
}
  return __read_alias (__fd, __buf, __nbytes);
}


That is, if __nbytes is greater than the result of GCC's built-in object size 
(https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html) for the struct, 
we'll kick back a warning.

As it turns out, this is because there is an error in the code here - we're 
passing the address of the pointer to the struct, not iev, which is a pointer 
to the struct. Hence, the number of bytes is probably going to be lot larger 
than the number of bytes that make up a pointer! Changing this to just read 
from the pointer to the struct fixes the warning.


- Matt


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


On March 29, 2015, 8:52 p.m., Diederik de Groot wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4530/
> ---
> 
> (Updated March 29, 2015, 8:52 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

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

[Diederik de Groot]

> On March 30, 2015, 4:11 a.m., Matt Jordan wrote:
> > So, small problem with this change. When compiling with gcc 4.8.2, I get 
> > the following warning:
> > 
> > In function ‘read’,
> > inlined from ‘inotify_daemon’ at stdtime/localtime.c:380:6:
> > /usr/include/x86_64-linux-gnu/bits/unistd.h:42:2: error: call to 
> > ‘__read_chk_warn’ declared with attribute warning: read called with bigger 
> > length than size of the destination buffer [-Werror]
> >   return __read_chk_warn (__fd, __buf, __nbytes, __bos0 (__buf));
> > 
> > 
> > Interestingly, the build agents didn't kick this back. But we'll need to 
> > find a way to get this fixed for gcc as well.
> 
> Matt Jordan wrote:
> So, this is interesting. Looking at unistd.h:
> 
> extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
>size_t __buflen) __wur;
> extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
>   size_t __nbytes), read) __wur;
> extern ssize_t __REDIRECT (__read_chk_warn,
>(int __fd, void *__buf, size_t __nbytes,
> size_t __buflen), __read_chk)
>  __wur __warnattr ("read called with bigger length than size of "
>"the destination buffer");
> 
> __fortify_function __wur ssize_t
> read (int __fd, void *__buf, size_t __nbytes)
> {
>   if (__bos0 (__buf) != (size_t) -1)
> {
>   if (!__builtin_constant_p (__nbytes))
> return __read_chk (__fd, __buf, __nbytes, __bos0 (__buf));
> 
>   if (__nbytes > __bos0 (__buf))
> return __read_chk_warn (__fd, __buf, __nbytes, __bos0 (__buf));
> }
>   return __read_alias (__fd, __buf, __nbytes);
> }
> 
> 
> That is, if __nbytes is greater than the result of GCC's built-in object 
> size (https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html) for the 
> struct, we'll kick back a warning.
> 
> As it turns out, this is because there is an error in the code here - 
> we're passing the address of the pointer to the struct, not iev, which is a 
> pointer to the struct. Hence, the number of bytes is probably going to be lot 
> larger than the number of bytes that make up a pointer! Changing this to just 
> read from the pointer to the struct fixes the warning.

I guess i was running into the same problem whem running the tests/test_time.c 
using the clang compiled version. It segfaulted in inotify_daemon as wel. 
Thanks for finding/fixing this one. Really making progress here.


- Diederik


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


On March 30, 2015, 3:52 a.m., Diederik de Groot wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4530/
> ---
> 
> (Updated March 30, 2015, 3:52 a.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