Fix for LP32 platforms: convert pointers via uintptr_t to/from integral types

[fbrausse]

Hi,

I'm using facil.io on a i686 Linux machine (gcc-9.3.0) and noticed lots of warnings about casting pointers to integers of different widths, e.g. (uint64_t)&obj, an (undefined) left-shift of a 32-bit size_t by 33 in the risky hash computation, as well as the explicit use of sendfile64 with an off_t parameter.

This patch fixes these warnings by

• casting pointers to uintptr_t first,
• using an intermediate uint64_t for the risky hash mixing in of size_t len, and
• unconditionally adding the _FILE_OFFSET_BITS=64 #define to the FLAGS in makefile.

The API has not been touched, just the internal calls. This has been the most obvious way for me to fix compilation of facil.io on this 32-bit platform, though I'm open to fixing these in a different way if you have any suggestions.

[boazsegev]

Hi @fbrausse ,

Thanks for the PR! Could you add a note in the CHANGELOG and credit yourself for the PR? (like the one added here).

I already implemented some of the proposed changes in one of the 0.8.0.beta repositories (this is just a single header library with the core modules that could be isolated from the IO logic)... I should have thought to test the code on 32 bit platforms, but all the machines I run my code on are 64 bit 🤷‍♂️

Anyway, thanks a lot!

Bo.

P.S.

If there's anything you want to see in the next version (where breaking changes to the API will occur), let me know.

I hate breaking the API, but 0.7.x is such a patchwork of ideas and the API isn't as unified and coherent as I'd like. I'm trying to smooth out API inconsistencies - i.e., fio_str_free should really have matched fiobj_free's behavior rather than having it behave the same as fiobj_str_resize(str, 0)... so this is a good time to voice out any comments / ideas.

[fbrausse]

Hi Bo,

I've added the Changelog entry as requested and expanded detail in the commit message, the rest of the patch remains unchanged.

Thank you for all your work on these libraries and APIs in particular - we don't need Python to have simple interfaces! :)

Regarding API consistency, I'm afraid there are no suggestions I can give you right now. However, for ref-counted types (like fio_str) I agree that *_free() should behave like the counter-part to *_dup() as in C (nearly) all we have are naming conventions.

Personally, I've adopted the convention of ref()/unref() pair for ref-counted types (from GObject, I believe) vs. the init()/fini() pair for types with directly managed lifetimes to be created on the stack. My heap-allocated types (by create()/destroy() for alloc + init) usually are not ref-counted. If they are, I use create()/ref()/unref() without destroy().

Kind regards,
Franz

[codecov-io]

Codecov Report

Merging #96 into 0.7.x will increase coverage by 0.26%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##            0.7.x      #96      +/-   ##
==========================================
+ Coverage   50.21%   50.48%   +0.26%     
==========================================
  Files          33       33              
  Lines       14365    14382      +17     
==========================================
+ Hits         7214     7261      +47     
+ Misses       7151     7121      -30     

Impacted Files	Coverage Δ
lib/facil/cli/fio_cli.c	0.00% <ø> (ø)
lib/facil/fio.c	59.82% <0.00%> (+0.02%)	⬆️
lib/facil/redis/redis_engine.c	0.00% <0.00%> (ø)
lib/facil/fiobj/fiobject.h	78.99% <50.00%> (+19.37%)	⬆️
lib/facil/fio.h	83.69% <100.00%> (+1.64%)	⬆️
lib/facil/fiobj/mustache_parser.h	74.83% <0.00%> (-0.44%)	⬇️
lib/facil/http/http.c	1.43% <0.00%> (-0.01%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01f40f5...120cc37. Read the comment docs.

[boazsegev]

Great! thanks!