search

ThePhD / future_cxx

Work done today for the glory of tomorrow - or, C and C++ systems programming papers.
https://thephd.dev/portfolio/standard
46 stars 8 forks source link

N2730: Parameter order discrepancy in §3.5 vs other places #42

Closed alexshpilkin closed 2 years ago

alexshpilkin commented 2 years ago

In §3.5 (Prior Art, iconv/ICU) the general function signature is given as (address then length, input then output)

stdc_mcerr stdc_XstoYs(const charX** input, size_t* input_bytes, const charY** output, size_t* output_bytes);

whereas §4, all later discussion, and the actual implementation in cuneicode use (length then address, output then input)

stdc_mcerr stdc_XntoYn(size_t* output_size, charY** output, size_t* input_size, const charX** input); /* etc. */

This is probably a bug in the former. Now that I’m looking at this closely, the Xs and Ys are probably also leftovers from earlier versions of the paper (which had functions with mbs[r]towcs-like signatures) and should be changed into Xn and Yn.

I have checked that this is present in the latest Bikeshed source in the repo as well as the published N2730.

(I also think it would be nice to at least comment in §3 why the proposed API puts the length first, disagreeing with ICU, iconv, and almost every other standard C or POSIX API in existence, thereby nearly reaching −4 points on Rusty Russell’s scale for “Follow common convention and you’ll get it wrong”, but that may or may not be a bug and you may or may not wish to listen to me on it.)

alexshpilkin commented 2 years ago

Whoops. I just noticed that the paper’s §4 and proposed spec 7.S�.2 paragraph 11 (and only those) also declare const size_t* output_size and the const is surely an error (it doesn’t make sense and is absent elsewhere); I can file this separately if needed.

ThePhD commented 2 years ago

Yeah, these are both mistakes on my part, good catches.

Thank you, I'll fix them!

ThePhD commented 2 years ago

These should be fixed now!

alexshpilkin commented 2 years ago

Thank you for the quick response and happy New Year! :partying_face:

I see the proposed spec text has been fixed in r6, but §3.5 still seems to retains the opposite parameter order from everything else in both the Bikeshed source and the built HTML. Or am I looking in the wrong place?

alexshpilkin commented 2 years ago

Ah, I see: apparently I can’t read. Sorry.

For the record: Despite using the stdc_XstoYs name, the declaration in §3.5 is in fact describing the interface of iconv() and ucnv_{from,to}Unicode, which indeed use the parameter order given there. Tricky, but not an error, and probably the trickiness isn’t that problematic given this text is not going into the spec.