Test: add unit test for tor_htonll() and tor_ntohll()

Added in commit 9744a40f.

changed milestone to %Tor: 0.3.0.x-final

added component::core tor/tor easy milestone::Tor: 0.3.0.x-final points::0.2 priority::low resolution::implemented reviewer::dgoulet severity::minor status::closed test type::task labels

Heya - I added an attempt at this on the following branch:

https://github.com/mintytoast/tor-stuff/tree/bug_19563

One thing I noticed - in test_util_format.c there is a macro to define htonll:

#if !defined(HAVE_HTONLL) && !defined(htonll) #ifdef WORDS_BIGENDIAN #define htonll(x) (x) #else static uint64_t htonll(uint64_t a) { return htonl((uint32_t)(a>>32)) | (((uint64_t)htonl((uint32_t)a))<<32); } #endif #endif

Should the test case using the above be changed to use tor_htonll? If so I can do this in a separate patch.

Thank you in advance for your time in reading this comment.

Trac:
Username: icanhasaccount

Yes actually! That test file should definitely use tor_htonll(). Feel free to fix that in a separate commit.

About, your patch, looks good! I would simply test the edge cases that is the UINT64 max and 0. Also, I would define the expected "little endian" and "big endian" value as const variables instead of just "n" and then copy 4 times the expected value of converted n. It will just make things clearer and avoid us to typo anything.

Moving it to 030 milestone. Also, next time, set the patch to needs_review, will be easier for us to spot that it actually needs review :). Big thanks for this!

Trac:
Status: new to needs_revision
Milestone: Tor: 0.2.??? to Tor: 0.3.0.x-final
Reviewer: N/A to dgoulet

Thank you - I've updated the branch re the feedback. I'll open another bug (with a patch) for test_util_format.c.

Thanks again!

Trac:
Username: icanhasaccount
Status: needs_revision to needs_review

Oh it's fine that you do the cleanup with this branch! :) I'll look at it asap.

Heya - thanks, I've put the fix for test_util_format into the same branch:)

Trac:
Username: icanhasaccount

lgtm and works for me! (well only tested on Little Endian :S). Thanks!

Trac:
Status: needs_review to merge_ready

merged!

Trac:
Resolution: N/A to implemented
Status: merge_ready to closed

closed

changed time estimate to 1h 36m

moved to tpo/core/tor#19563 (closed)