Re: Quiteing clang static analyzer

[Jerry Jacobs <jerryjacobs1989@...>]

> On 14 Mar 2017, at 20:44, Nagaev Boris <bnagaev@gmail.com> wrote:
> 
> Can you share why it was not done?
> 
> Adding __attribute__((noreturn)) to the one function seems better than
> adding forgotten returns to all the invocations. The only thing I am
> concerned about is a phrase from the manual:

Adding noreturn doesn’t resolve the issue I’m talking about. Keep in mind
 attributes are compiler specific. It is possible some compilers will not support
 them. A static analyzer doesn’t have to implement all compiler features which
 are not in the standard.

> 
>> it is an idiom to use it in C functions as return luaL_error(args).
> 
> which this code does not follow.
> 
> On Mon, Mar 13, 2017 at 9:48 PM, Igor Ehrlich <iehrlich@iponweb.net> wrote:
>> I actually understand why it was not done. That's a pity, sorry for the
>> noise.
>> 
>> On Tue, Mar 14, 2017 at 12:43 AM, Igor Ehrlich <iehrlich@iponweb.net> wrote:
>>> 
>>> I'll just cautiously metion that the analyzer warning is 100% correct from
>>> the language perspective. Should we possibly consider fixing it in the
>>> interpreter? The fix seems to be _rather_ straightforward, without any
>>> possibly negative performance effects.
>>> 
>>> On Tue, Mar 14, 2017 at 12:32 AM, Jerry Jacobs <jerryjacobs1989@gmail.com>
>>> wrote:
>>>> 
>>>> It hasn’t anything todo with the incorrectness of luaL_error. But with
>>>> the analyzer choking on a potential NULL pointer deference. The function
>>>> os_date returns an int. So does luaL_error return an int. On other places
>>>> return luaL_error(“”) is implemented, except for this place. Which would be:
>>>> 
>>>>  if (stm == NULL)  /* invalid date? *
>>>>    return luaL_error(L, "time result cannot be represented in this
>>>> installation”);
>>>> 
>>>> Its only necessary for the analyzer to shut its mouth, the code is 100%
>>>> correct.
>>>> 
>>>>> On 13 Mar 2017, at 22:25, Igor Ehrlich <iehrlich@iponweb.net> wrote:
>>>>> 
>>>>> I believe the proper way is to mark luaL_error with a no-return
>>>>> specifier? Something like GCC's "__attribute__((noreturn))" would possibly
>>>>> do the trick. Also, this the actual syntactic expression for "functions
>>>>> using setjmp/longjmp".
>>>>> 
>>>>> On Mon, Mar 13, 2017 at 11:54 PM, Jerry Jacobs
>>>>> <jerryjacobs1989@gmail.com> wrote:
>>>>> I’m using lua 5.3 in a project and clang static analyzer (scan-build)
>>>>> complained about a potential NULL pointer deference in OS lib (loslib.c).
>>>>> The analyzer doesn’t know luaL_error uses setjmp/longjmp and it has its full
>>>>> right to complain. The os_date C function contains this code:
>>>>> 
>>>>> stm = l_gmtime(&t, &tmr);
>>>>> 
>>>>> ~~~ snip ~~~
>>>>> 
>>>>>  if (stm == NULL)  /* invalid date? */
>>>>>    luaL_error(L, "time result cannot be represented in this
>>>>> installation”);
>>>>> 
>>>>> ~~~ snip ~~~
>>>>> 
>>>>> problem “passing NULL pointer” ->  setallfields(L, stm);
>>>>> 
>>>>> A simple solution would be just doing a return luaL_error which
>>>>> silences this false-positive.
>>>>> 
>>>>> Kind regards,
>>>>> Jerry Jacobs
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Best regards,
>>>>> Igor A. Ehrlich
>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Best regards,
>>> Igor A. Ehrlich
>> 
>> 
>> 
>> 
>> --
>> Best regards,
>> Igor A. Ehrlich
> 
> 
> 
> -- 
> Best regards,
> Boris Nagaev
>