od: fix handling of non-ascii chars

[tgrez]

Fixes #8202

I'm not sure if this fix is complete, because I couldn't reproduce the original behavior of uutils impl with GNU od, so I don't know in which situations the previous implementation was valid (I assume there is/was a reason for implementing it the way it was implemented).

Regardless of the above, this fixes the reported issue. It still lacks localization support, but it works correctly for LC_ALL=C as well as LC_ALL=en_US.UTF-8.

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

[RenjiSann]

It still lacks localization support

I'm not familiar with od, do you have an example of its behavior changing depending on the locale ?

[tgrez]

It still lacks localization support

I'm not familiar with od, do you have an example of its behavior changing depending on the locale ?

I'd say the difference could come from different encodings, eg:

$ echo -n 'ą' | iconv -f UTF-8 -t ISO-8859-2 | od -t c
0000000 261
0000001
$ echo -n 'ą' | od -t c
0000000 304 205
0000002

[tgrez]

And also:

$ echo -n 'ą' | LC_ALL=pl_PL.ISO-8859-2 od -t c
0000000   ? 205
0000002
$ echo -n 'ą' | LC_ALL=C od -t c
0000000 304 205
0000002

[RenjiSann]

I'd say the difference could come from different encodings, eg:

$ echo -n 'ą' | iconv -f UTF-8 -t ISO-8859-2 | od -t c
0000000 261
0000001
$ echo -n 'ą' | od -t c
0000000 304 205
0000002

Unless I'm mistaken, in this example, it's the input given to od that changes, so I don't think it changes anything for od in itself.

[RenjiSann]

I think this fix is good though, I'll happily merge it once the typo is addressed ^^'

[tgrez]

Unless I'm mistaken, in this example, it's the input given to od that changes, so I don't think it changes anything for od in itself.

Ah, yes, you're correct.

I'll happily merge it once the typo is addressed ^^'

Sorry for the confusion, which typo? :)

[RenjiSann]

Sorry for the confusion, which typo? :)

C.f. my comment #8268 (comment)

[tgrez]

Happy to adjust anything, including typos.

Sorry, but I can't see this comment, after I click the link I'm redirected only to this PR, adding some screenshots:

[RenjiSann]

Suggested change

	// // ASCII byte (0xxxxxxx)
	// ASCII byte (0xxxxxxx)

[RenjiSann]

That's absolutely my bad, I didn't see this was still pending
Sorry, thanks for noticing 😅

[tgrez]

That's absolutely my bad, I didn't see this was still pending Sorry, thanks for noticing 😅

No worries! And thank you for the review!

Fixed the typo, rebased on the latest main and retested.

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

[RenjiSann]

Thank you for your contribution ! 🚀