Test for the size of struct http_parser fails on 32 bit systems where there is padding/alignment for void*

[frmichael]

Bonjour,

Porting http-parser to AIX, where there are 32 and 64 bit environments, there is a test on sizeof (struct http_parser) which fails due to 8 byte reservation for void* on both 32 and 64 bit compiles.

A clean option might be to change the void data element of struct http_parser to be a union of uint64 and void.

But this may not work if the 32 bit size of struct http_parser must remain at a total of 28 bytes (and in which case the test is in correct).

However, supposing that this is an error in the test code, I provisionally used the following patch for the 32 bit build

--- ./test.c.ORIG       2020-11-27 22:02:53 +0100
+++ ./test.c    2020-11-27 22:57:18 +0100
@@ -4234,7 +4234,7 @@
   printf("http_parser v%u.%u.%u (0x%06lx)\n", major, minor, patch, version);
 
   printf("sizeof(http_parser) = %u\n", (unsigned int)sizeof(http_parser));
-  assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *));
+  assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + 2*sizeof(void *));
 
   //// API
   test_preserve_data();

We would like to ask for your expert advice on how best to resolve this issue.

Thanks.

[frmichael]

The patch has been refined to apply to 32 and 64 bit builds (feedback appreciated)

--- ./test.c.ORIG       2020-11-30 14:17:06 +0100
+++ ./test.c    2020-11-30 14:21:43 +0100
@@ -4234,7 +4234,11 @@
   printf("http_parser v%u.%u.%u (0x%06lx)\n", major, minor, patch, version);
 
   printf("sizeof(http_parser) = %u\n", (unsigned int)sizeof(http_parser));
+#if defined(_AIX) && !defined(__64BIT__)
+  assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + 2*sizeof(void *));
+#else
   assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *));
+#endif
 
   //// API
   test_preserve_data();

[pallas]

FYI, http-parser is dead.. long live https://github.com/nodejs/llhttp

[cbiedl]

@frmichael: It seems you did include 4f15b7d since it disabled that assertion check for non-x86?

Still, that fix is incomplete as it still breaks on i386 (at least in Debian) where sizeof(void *) ==4 and therefore sizeof(http_parser) just 28. Ironically, comparing to the old 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *) makes the check pass.

[cbiedl]

@frmichael: Sorry, forgot a "not" in my previous comment.

Also my suggested fix does not work for x32. le sigh.

[pallas]

Again, I'd strongly suggest switching to nodejs/llhttp since it should be a mostly drop-in replacement. It's generated by llparse, but you can get the pre-compiled source from the release branch, i.e. https://github.com/nodejs/llhttp/tree/release

[cbiedl]

Derrick Lyndon Pallas wrote...

Again, I'd strongly suggest switching to nodejs/llhttp since it should be a mostly drop-in replacement. It's generated by llparse, but you can get the pre-compiled source from the release branch, i.e. https://github.com/nodejs/llhttp/tree/release

While I agree there is no future for http-parser, also the sources are rather a nightmare and I'd be happy to replace it with something more robust ...

Life is not that easy. There's a lot of code out there that was written with http-parser in mind. But llhttp is not just another project, another build system - but also another philosophy. My first attempt to use it ended in huge pile of node.js error message, an area where I'd have to build up some knowledge first. After that experience I could not check how difficult the transition actually would be; however by comparing the .h files I reckon llhttp is not just a simple drop-in replacement, as in: At most change the #include setting and you're done.

All this will make people stick to http-parser way longer than sensible and desirable. If you want to speed up the procedure, I suggest you start helping people with the switch. That's more work than than just advocating, though.

For example, bringing llhttp to the Debian (and derivatives) ecosystem was a huge step forward (that's https://bugs.debian.org/977716, note that using the pre-compiled C files is not an option). Having extra documentation about how to adjust an application (a.k.a. Rosetta Stone) was nice to have. And as a last refinement, a compatibility library that translates the old library functions into the new ones, or at least those 90 percent that are most commonly used.