On Sat, Mar 23, 2013 at 09:27:50AM -0500, Vladimir Támara Patiño wrote:
> >Please change this to look like
> >in other parse.y files in the OpenBSD src tree (e.g. look at bgpd).
> 
> Could you change?

No, sorry. I don't have time for doing that.
Please cross-check the parse.y files yourself and make sure the new
parse.y file follows conventions used in the existing parse.y files.
We want our source tree to be consistent.

I see you're still tempted to copy code from FreeBSD without making
it perfect for OpenBSD. I'm not going to allow you to be that lazy ;-)

> The attached patch only adds LC_COLLATE for existing locales (single
> byte or translatable to ISO8859-1).

The new diff looks much better already.

Why do you compile colldef with COLLAGE_DEBUG by default?

> In case you want to give me credit in the CVS log, could you please use
> my email?

Sure. But I don't know yet if/how this is going to be committed.
I think it makes sense to put the libc bits and usr.bin parts in at
the same time. Don't expect me to hunt for additional opinions
and OKs before I've gotten around to review the libc changes you
sent as well, and before I'm perfectly happy with both diffs.

I'm certainly not going to commit any of this without asking others
for review as well, so please don't be worried if it takes some time
to get this in. I was working on and off on UTF-8 locale support over
the course of two years -- these things can take some time, and I'm not
going to rush things unnecessarily. But I think you're on the right track.

collate.h has this:

> +#define STR_LEN 10

which is a very bad generic name.
This should be named something like COLLATE_MAX_STR_LEN.

Also, the collate.h file has:

> +__BEGIN_DECLS
> +#ifdef COLLATE_DEBUG
> +void __collate_print_tables(void);

This is bogus. colldef.c declares and defines a collate_print_tables()
function. The __collate_print_tables() function is used in the libc
diff you sent, so it doesn't belong in the colldef diff.

> +#endif
> +__END_DECLS
> +
> +#endif /* !_COLLATE_H_ */
> 

Reply via email to