lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


Also, that's the actual benefit of the continuous usage of static analyzers :)

On Tue, Mar 14, 2017 at 10:44 PM, 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:

> 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




--
Best regards,
Igor A. Ehrlich