This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Add localedef --big-endian and --little-endian options


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]