Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time parsing 32bit corner-case #846

Open
matteo-cristino opened this issue Apr 5, 2024 · 0 comments
Open

Time parsing 32bit corner-case #846

matteo-cristino opened this issue Apr 5, 2024 · 0 comments

Comments

@matteo-cristino
Copy link
Collaborator

Since time is an integer, it could take values from -2147483648 to 2147483647, but if I try to do

TIME.new(-2147483648)

the following error is returned

Could not read unix timestamp -2.147484e+09

While this error is not returned in the case I use

TIME.new("-2147483648")

This seems due to some conversion from number to string done internally in lua since, as stated in the documentation, both number and strings are seen as string from lua_isstring function (making also the lua_isnumber branching useless)

Zenroom/src/zen_time.c

Lines 74 to 121 in ca02f6c

ztime_t* time_arg(lua_State *L, int n) {
Z(L);
ztime_t *result = (ztime_t*)malloc(sizeof(ztime_t));
if(result == NULL) {
return NULL;
}
void *ud = luaL_testudata(L, n, "zenroom.time");
if(ud) {
*result = *(ztime_t*)ud;
goto end;
}
if(lua_isstring(L, 1)) {
const char* arg = lua_tostring(L, 1);
char *pEnd;
long l_result = strtol(arg, &pEnd, 10);
if(*pEnd) {
free(result);
lerror(L, "Could not read unix timestamp %s", arg);
return NULL;
} else if (l_result < INT_MIN || l_result > INT_MAX) {
free(result);
lerror(L, "Could not read unix timestamp %s out of range", arg);
return NULL;
}
*result = (ztime_t)l_result;
goto end;
}
// number argument, import
if(lua_isnumber(L, 1)) {
lua_Number number = lua_tonumber(L, 1);
*result = (int)number;
goto end;
}
octet *o = o_arg(L, n);
if(o) {
if(o->len != sizeof(ztime_t)) {
free(result);
zerror(L, "Wrong size timestamp %s", __func__);
return NULL;
}
memcpy(result, o->val, sizeof(ztime_t));
o_free(L, o);
goto end;
}
end:
if(result) Z->memcount_times++;
return result;
}

and thus they both pass thorugh the lua_tostring function. I tried also to move the branch with lua_isnumber before the lua_isstring one, but it seemed that after a certain number, lua was not able to read them correctly.

Thus at the moment, if we use numbers in input we are only loosing the time -2147483648 and using strings in input we should not lose anything. Anyway interesting to notice and maybe look further into it in case any other problems with numbers came up.

@jaromil jaromil changed the title Time parsing not fully working Time parsing 32bit corner-case Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant