Re: [PATCH 2/3] CLD: switch network proto from UDP to TCP

[Jim Meyering <jim@meyering.net>]

Pete Zaitcev wrote:
> On Fri, 31 Dec 2010 05:57:28 -0500
> Jeff Garzik <jeff@garzik.org> wrote:
>
>> +	struct cldc_tcp *tcp = private;
>> +	ssize_t rc;
>> +	struct ubbp_header ubbp;
>> +
>> +	memcpy(ubbp.magic, "CLD1", 4);
>> +	ubbp.op_size = (buflen << 8) | 1;
>> +#ifdef WORDS_BIGENDIAN
>> +	swab32(ubbp.op_size);
>> +#endif
>> +
>> +	rc = write(tcp->fd, &ubbp, sizeof(ubbp));
>
> Why not this:
>
> 	unsigned int n;
>
> 	n = (buflen << 8) | 1;
> 	ubbp.op_size = GUINT32_TO_LE(n);

Nice.
Avoiding those pesky in-function #ifdefs makes the code more readable.

IMHO, this is kinder still on the eyes of reviewers, since the
types of "n" and "rc" stay even closer to each definition/first-use:

  	unsigned int n = (buflen << 8) | 1;
  	ubbp.op_size = GUINT32_TO_LE(n);

        ssize_t rc = write(tcp->fd, &ubbp, sizeof(ubbp));

But if your coding guidelines require the all-vars-decl'd-at-top style
(seriously anachronistic and more prone to merge conflicts), then I guess
that's not an option...  Though you may want to reconsider the policy,
if your goal is portability:

    For the record, compilers that reject decl-after-stmt are not
    relevant any more.  At least, there are so few that I've been
    able to keep that sort of construct in coreutils for the past several
    years.  I used to maintain a c99-to-c89 patch, but removed even that
    almost two years ago.