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

Homogenized format for tradspool tokens, independently of 32 or 64-bit archs #290

Open
Julien-Elie opened this issue Dec 22, 2023 · 0 comments
Labels
bug Something isn't working C: storage Related to storage methods P: low Low priority

Comments

@Julien-Elie
Copy link
Contributor

Julien-Elie commented Dec 22, 2023

Christoph Biedl reports that tradspool tokens have a different syntax on 32 and 64-bit architectures. Article numbers (as unsigned long in the tokens) are written either on 4 or 8 bytes, leading for instance to either @05630000135A000062850000000000000000@ or @05630000135A000000000000628500000000@.

The underlying issue is the assumption sizeof(uint32_t) == sizeof(long) which holds for 32 bit. But here, MakeToken gets an article number (artnum) as unsigned long (64 bit), converts it using htonl which is 32 bit, likewise for (group) num. For storing the article number however, sizeof(num) is used as the offset, so 8, while by design it should be only 4. Luckily, the reverse operation TokenToPath uses the same semantics, so things do work as expected, and nobody noticed in decades. Only tradspool_explaintoken fails since it uses uint32_t which follows the design but does not match the implementation.

There are several ways to get out of this. Quick and dirty, adjust tradspool_explaintoken and the documentation. Nicer was to make the implementation follow the design, replace unsigned long with uint32_t when it's about article and group numbers, and have a transition in the lookup code for the article number: it would check both offsets (4 and 8), the non-zero one has the actual information. That might as well be done as part of an upgrade.

@Julien-Elie Julien-Elie added bug Something isn't working C: storage Related to storage methods P: low Low priority labels Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C: storage Related to storage methods P: low Low priority
Development

No branches or pull requests

1 participant