webdis icon indicating copy to clipboard operation
webdis copied to clipboard

Update jansson to v2.1.4

Open koenvandesande opened this issue 2 years ago • 2 comments

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

koenvandesande avatar Aug 30 '23 18:08 koenvandesande

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 avatar Aug 30 '23 22:08 nicolasff

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.

nicolasff avatar Aug 31 '23 00:08 nicolasff

@koenvandesande I've merged your commits to main, so will close this PR shortly. They were cherry-picked to a branch to prepare the change and either make minor changes or document the differences between the original source code and what Webdis actually embeds.

I will try to tag a new release of Webdis soon, with these changes in.

Thanks again!

nicolasff avatar Oct 12 '24 02:10 nicolasff

@koenvandesande Webdis 0.1.23 is out, with your changes integrated. Images are published on:

I'll close this ticket shortly.

nicolasff avatar Oct 23 '24 04:10 nicolasff