[illumos-Developer] Problems in setlocale and codereview

[Garrett D'Amore]

On Thu, 2010-09-02 at 20:53 +0200, Joerg Schilling wrote:
> "Garrett D'Amore" <garrett at damore.org> wrote:
> 
> > Ok, I'll try to work on this today... unless you want to drive forward
> > with it. (Testing being the key missing requirement. :-)
> 
> After doing some typo fixes, it compiles and works correctly for my tests.

That's great news!

> 
> AFAIK, I have no permission to put things back to the illumos hg.

Right.  I'll give you the permission to do so, once you and I have a
chat, and I've verified that your integration is well-formed.  You will
need to make sure you have your public ssh key in your illumos account
though, since that's what we use to enable the permission.

> 
> I have some questions:
> 
> -	How do I apply my delta to my local repository?

I'm not sure I understand what you mean by this question.  If you've
already edited the code, you can commit locally using "hg commit".

> 
> -	Am I correctly asuming that (once I have the needed permission)
> 	I will have to call "hg push" in order to forward my local putback
> 	to the official repository?
> 
> -	How is the mail address from what I see with "hg log", e.g.:
> 
> 	changeset:   13162:61ea578f8715
> 	user:        Garrett D'Amore <garrett at nexenta.com>
> 	date:        Mon Aug 30 23:48:20 2010 -0700
> 	summary:     7 Need locale databases
> 
> 	derived? 

Its taken from ~/.hg/hgrc.  What you should do is
run /opt/onbld/bin/hgsetup which sets up a template file for you.

> 
> -	What email would be better?
> 
> 	joerg.schilling at fokus.fraunhofer.de	(work)
> 	or
> 	joerg at schily.net			(private)

Whichever you prefer.   If your employer owns the copyright, it would be
best to integrate using an email address @ your employer, but that's
been you and your employer. :-)

> 
> 
> > I'd actually love it if you could test and ultimately drive to
> > integration.  It could represent your first direct integration into
> > illumos.
> 
> If it is OK to first deactivate the problematic code, e.g. this way:
> 
>                 } else { 
> #ifndef __ready__ 
>                         errno = EINVAL; 
>                         return (NULL); 
> #else 
>                         char    *buf; 
>                         char    *save; 
>  
>                         buf = alloca(strlen(locale) + 1); 
> 
> I see no problem to integrate the current code.

Let me review the code in more depth.  I've been in meetings all morning
and haven't had a chance to read the actual code yet.

	- Garrett