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

arity: short, int, or size_t #1337

Open
mgondan opened this issue Dec 16, 2024 · 4 comments
Open

arity: short, int, or size_t #1337

mgondan opened this issue Dec 16, 2024 · 4 comments

Comments

@mgondan
Copy link
Contributor

mgondan commented Dec 16, 2024

I encountered a runtime warning from a sanitizer for undefined behavior when I registered a foreign c function, complaining about a wrong type for the arity of my function. This seems to be inconsistent in the code (?)

pl-vmi.c, 4477: VMH_GOTO_AS_VMI(I_FEXITDET, (*f)(h0, DEF->functor->arity, &FNDET_CONTEXT));
pl-wrap.c, 258: size_t
pl-termhash.c, 525: int
foreign.doc: short

What is the correct type, size_t?

@JanWielemaker
Copy link
Member

Historically, arity was int. At some time it was converted to size_t. The pl-termhash.c is wrong (but rather benign; it is only a hash, now computed from arity modulo 2^32). the definition in foreign.doc is fine, as predicates are limited to 1024 arguments. Of course, there may be some dubious conversion somewhere ...

@mgondan
Copy link
Contributor Author

mgondan commented Dec 17, 2024

I see. Would you mind a few PRs? I see many warnings if I compile with -Wconversion. I guess 90% are easily solved using e.g. size_t, but a few of them may need auditing.

@JanWielemaker
Copy link
Member

I see. Would you mind a few PRs?

I wouldn't mind getting all this in sync. Be careful though, size_t is unsigned, where int is signed. This can break some code if you just change the type. There are probably also structs where we can safely assume the represented number of arguments to fit in a smaller type and use this to reduce the size.

If I recall well, I did a lot of these. Do them in small chunks, commit and run ctest frequently. That should capture most issues.

... actually, I'm keener getting bool being used consistently throughout 😄 Mostly because it helps a lot examining the code if you know a function is bool or int and (thus) has more than two possible result states.

@mgondan
Copy link
Contributor Author

mgondan commented Dec 19, 2024

I see your point. Many warnings arise indeed from return values of different types.

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

2 participants