[dropped Florian from CC] Hi Alex, I have some feedback on project management. And then a couple of minor technical points. At 2025-01-20T23:26:04+0100, Alejandro Colomar wrote: [much snipped] > I'd rename inbuf,outbut to in,out. > > > + char *locale_pathname; > > + char *outbuf; > > + FILE *fp; > > + size_t iconv_result; [...] > I've removed this variable (see below). [...] > Please align (and merge some) with spaces the above as: > > char *locale_pathname; > char *in, *out; > FILE *fp; > size_t size; > size_t inbytes, outbytes; > iconv_t cd; > > I've also reordered a few so that they appear in order of use (more or > less). [...] > Please use sizeof(utf32_pathname), with parentheses. [...] > This variable seems useless: > > if (iconv(cd, &in, &inbytes, &out, &outbytes) == \-1) > > > + err(EXIT_FAILURE, "iconv"); > > +\& > > + if (iconv_result == \-1) > > + err(EXIT_FAILURE, "iconv"); > > This is a leftover from the previous version. [even earlier] > Please group declarations of the same type in consecutive lines. > Shorter type names up and longer type names below. For same length, > please use alphabetic order. This style of feedback is producing a lot of churn. Jason's going to be well into the v-teens before this patch is accepted, at this rate. It appears to me that what is happening here is that you are iteratively developing a C code style guide under the banner of a man page review. If Jason's okay with being the test subject for this procedure, then there's not exactly a problem here, but if it were me submitting a man page, I'd be getting frustrated by (or before) this point. I just "git pulled" https://git.kernel.org/pub/scm/docs/man-pages/man-pages/ and checked "./man/man7/man-pages.7", and practically _none_ of the rules you've been stating to Jason is expressed there. I propose that the submissions of patches to the Linux man-pages not be a black-box process, with you serving as the oracle that accepts or rejects the input. I admit you're offering a bit more information than a binary semaphore (ACCEPT/REJECT), but still, it would be better if Jason, and others, had a "Linux man-pages example C code style guide" document they could consult so that they could anticipate more of your objections. If the construction of such a document is what this precise instance of the process is groping toward, good. If not, then I suggest that it's about time to prepare that document. I don't dispute that having a consistent style for code examples in the Linux man-page corpus is worthwhile; I do think it will, ultimately, pay dividends to harried hackers in a hurry. But it is precisely to the extent that style guidelines are arbitrary that they need to be documented and easily located. On different, nerdier subjects... > Please don't use braces for a single statement. I think they are helpful for clarity. Yes, modern compilers will warn about misleading indentation. I still think braces around any block guarded by control instructions are a good idea for the human brain interpreting code. And the presence of the braces costs nothing at translation time. Does any compiler construct a new stack frame just because it saw an opening brace in the input (that wasn't part of an initializer)? > Please separate declarations from code. I think this is considered old-fashioned in some quarters. It has been valid since ISO C99 to introduce declarations anywhere, and a common style is to place them at, or adjacently to, the point where they're used. The traditional arrangement of placing all declarations at the top of a function definition arises, as I understand it, from the limitations of early compilers, which were often--and sometimes had to be--simple and small. When the compiler read the function definition, it could generate an assembly language preamble for setting up a stack frame that reserved all of the room necessary for any storage of automatic duration, and then start translating statements into instructions at once.[1] (A test of this understanding would be whether any pre-C99 compilers rejected "late" declaration of automatic variables, but happily accepted them for static or register variables, since those would not complicate stack memory allocation. I'm not quite old enough to say; for the first <mumble> years of my programming career, GCC was the only C compiler I ever used.) Anyway, this is another of those matters of taste, so if mandatory early declarations are to be the rule, you probably want to say so explicitly so that you're not mistaken for a grognard who either isn't aware that ISO C99 happened, or, like Dennis Ritchie, refused to countenance its its existence with a 3rd edition of its central textbook, and eventually ran out of time to do so. (In a 2000 interview, he said it "needs to quiesce for a while".)[2] Finally, I'll note that asserting a dichotomy between "declarations" and "code" can be misleading. Declarations can generate assembly language too, and not just when they are coupled with initializers. I'd say "declarations" and "statements" instead, or avoid the issue entirely and say something like, "group all variable declarations at the top of each function". Regards, Branden [1] This is also borne out by other structural features of pre-ANSI C function definitions. Return type first because the corresponding value will need to be visible in the enclosing scope. Then, _outside_ the function parameter parentheses, the types of any arguments the function takes, because they'll be pushed onto the stack before the function is called. Then, inside the corresponding assembly subroutine, stack memory is set aside to house whatever local--meaning non-static, non-register, storage is needed. Maybe if I actually wrote a compiler for pre-ANSI C, I'd know for sure. ;-) [2] http://www.gotw.ca/publications/c_family_interview.htm
Attachment:
signature.asc
Description: PGP signature
