Re: CVS commit: src

[Lourival Vieira Neto <lneto%netbsd.org@localhost>]

Hi Alexander,

> 1. This doesn't look right:
>
> #define luai_numpow(a,b)        luai_nummul(a,b)

Yes, it isn't. But, please note, I didn't change that now. I just
merged it in one single file. Though I think we need implement integer
exponentiation, I think that is not a priority. IMO, it is a TODO.

> 2. If intmax_t is a greater type than int64_t, the below doesn't
> handle overflow:
> #define lua_str2number(s,p)    ((int64_t) strtoimax((s), (p), 10))

I'm considering two approaches:

1) creating an auxiliary function based on strtoimax():

int64_t
strtoint64(const char *nptr, char **endptr, int base)
{
    imax_t imax = strtoimax(nptr, endptr, base);
    return (int64_t) (imax >= INT64_MIN && imax <= INT64_MAX ? imax : 0);
}

#define lua_str2number(s,p)    (strtoint64((s), (p), 10))

2) creating an auxiliary function based on strtol template

(...)
#define _FUNCNAME strtoint64
#define __INT int64_t
#define __INT_MIN INT64_MIN
#define __INT_MAX INT64_MAX

#include "_strtol.h"
(...)

I prefer the first approach, because it uses a std function without
re-implementing it. Waiting to hear your and others considerations.

> I checked Lua code and there is no overflow check. However, I noticed
> that they detect "0x" strings and convert them using strtoul. It should
> be changed to stroumax. This is in function luaO_str2d in src/lobject.c.

I'll check that.

> 3. Item 1 was in my earlier review of luaconf.h in sys/modules and I see
> at least one minor item not covered by your new change. Can you please
> go over my review and make sure all covered?

Sorry, I've missed that. Which review? Are you talking about the
"changing lua_Number to int64_t" thread?

Regards,
-- 
Lourival Vieira Neto