This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Add localedef --big-endian and --little-endian options
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Mon, 23 Sep 2013 15:51:13 -0700 (PDT)
- Subject: Re: Add localedef --big-endian and --little-endian options
- Authentication-results: sourceware.org; auth=none
- References: <Pine dot LNX dot 4 dot 64 dot 1309112327530 dot 27435 at digraph dot polyomino dot org dot uk>
This seems fine modulo some nits I noted below. I didn't carefully check
the patch or the log entry for omissions or whatnot--I trust your testing.
> (options): Add --little-endian and --bit-endian options.
typo
> +* The localedef utility now supports --big-endian and --little-endian
> + configure options to generate locales for a different system from that for
s/configure/command-line/
> + which glibc was built.
s/glibc/the C library/
> + { "little-endian", OPT_LITTLE_ENDIAN, NULL, 0,
> + N_("Generate little-endian output") },
> + { "big-endian", OPT_BIG_ENDIAN, NULL, 0, N_("Generate big-endian output") },
Wrap the second line the same just for visual consistency.
> +/* Get and set values (possibly endian-swapped) in structures mapped
> + from or written directly to locale archives. */
> +#define GET(VAR, FIELD) maybe_swap_uint32 ((VAR).FIELD)
> +#define SET(VAR, FIELD, VALUE) ((VAR).FIELD = maybe_swap_uint32 (VALUE))
Why are VAR and FIELD two arguments here?
There's no benefit over a macro that just takes VAR.FIELD as one argument.
> +/* True if the locale files use the opposite endianness to the
> + machine running localedef. */
> +int swap_endianness_p;
Use bool.
> +static inline void
> +set_big_endian (int big_endian)
Take bool argument.
Thanks,
Roland