search

nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

Update jansson to v2.1.4 #239

Open koenvandesande opened 10 months ago

koenvandesande commented 10 months ago

The bundled version is over 13 years old. This updates it.

nicolasff commented 10 months ago

Hi Koen,

Thanks for the PR! You're right, 13 years is pretty long and it's probably time to upgrade :-) I'll take a look at the code scanning results, nothing looks too relevant although the unused function does produce a warning so I'll take care of that. I'll have to see if I can avoid it spamming PRs so much, as well.

There are also a bunch of warnings about the use of snprintf copying strings into buffers for which the size is not entirely clear to the compiler, I'll also look into those. The build output with these warnings is pretty verbose, so I've uploaded it to this Gist.

There is a more serious issue with this code though, it does not build on macOS. This is because src/jansson/src/lookup3.h includes endian.h, which is usually not present on this OS. It looks like that can just be replaced with <machine/endian.h> though, I'll give it a shot.

I see you've already removed a couple of unused files, thanks for that. My plan for now is:

  1. to see if there are more files that can be removed
  2. to address the errors and warnings seen during the build
  3. and to document all of these changes in a new file under src/jansson to help with future upgrades.

If all goes well I can do all this in one commit on top of your existing two, and merge that. I'll give you and update shortly.

nicolasff commented 10 months ago

Ok, I've pushed it to https://github.com/nicolasff/webdis/tree/update_jansson_rebased. I ended up fixing the commit title typo in f5624cf975223a326eed45d26d909df766c55028, preserving your authorship of the commit (see 21060c9b1ef9be1d5a78795ec99eedbb53d46023).

The list of changes is documented here, which will be useful for future upgrades: https://github.com/nicolasff/webdis/blob/update_jansson_rebased/src/jansson/WEBDIS-CHANGES.md

I'll run some tests, both those from the repo and some extra validation scripts that check for memory leaks. If all looks good I can merge this new branch soon.