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

Name collisions for functions without prefix #754

Open
KaruroChori opened this issue Dec 14, 2024 · 6 comments
Open

Name collisions for functions without prefix #754

KaruroChori opened this issue Dec 14, 2024 · 6 comments

Comments

@KaruroChori
Copy link
Contributor

KaruroChori commented Dec 14, 2024

There are several functions in qjs which are too easy to collide as:

  • they are not static
  • they are not namespaced with a prefix.

There are two specific offenders I encountered, pstrcat & pstrcpy, both colliding with their equivalent implementations in tinycc. There are few more which have just too much of a generic name, like has_suffix.
All these "offenders" are declared in cutils.h and implemented in cutils.c.

My suggestion would be for these functions to be scoped with some prefix, as I don't think they are really meant to be used from outside qjs.

@saghul
Copy link
Contributor

saghul commented Dec 14, 2024

They are not zoomed in the API so their visibility should be hidden, how are you using them?

@chqrlie
Copy link
Collaborator

chqrlie commented Dec 14, 2024

I ran into the same issue for qemacs, for the same reason... as the original author of these functions that Fabrice collected in cutils.[hc] and used in different projects. We should just scope them with a js_ prefix using #define or changing the calls in the qjs source files (actually very few instances). Other offenders include strstart, rqsort, [ui]{32,64}toa{,_radix} and digits36, utf8_xxx handlers, dbuf_xxx API...

@KaruroChori
Copy link
Contributor Author

They are not zoomed in the API so their visibility should be hidden, how are you using them?

Just statically linking qjs and tcc within the same project.

@saghul
Copy link
Contributor

saghul commented Dec 14, 2024

👍 would either of you mind sending a pr?

@chqrlie
Copy link
Collaborator

chqrlie commented Dec 14, 2024

👍 would either of you mind sending a pr?

Will do! back in business :)

KaruroChori pushed a commit to KaruroChori/quickjs that referenced this issue Dec 15, 2024
@KaruroChori
Copy link
Contributor Author

KaruroChori commented Dec 15, 2024

I already started changing some locally to make my project work

void js__pstrcpy(char *buf, int buf_size, const char *str);
char *js__pstrcat(char *buf, int buf_size, const char *s);
int js__strstart(const char *str, const char *val, const char **ptr);
int js__has_suffix(const char *str, const char *suffix);

and pushed them in #755

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

3 participants