Brock Pytlik wrote:

> On 05/ 3/11 02:12 PM, Danek Duvall wrote:
> >Brock Pytlik wrote:
> >
> >>http://cr.opensolaris.org/~bpytlik/ips-18240-v1/
> >A handful of quick nits:
> >
> >Be sure to update copyright dates -- I see a lot of 2010.
> Well, at lot of these things were written in 2010 and haven't been
> changed since. I thought I remembered you having a similar situation
> and leaving the copyrights @ 2010. If I've misunderstood, I'm happy
> to change them.

Okay, if they haven't changed, that's fine.

> >   - this mentions a zoneproxy-client(1M) manpage, but you don't have one.
> >
> Yep, I'll file a bug to add a man page or I can remove the mention.

Filing the bug is fine, as long as it gets written prior to FCS.

> >zoneproxy/list/Makefile:
> >
> >   - Why all these extra CFLAGS?
> They matched what it was compiled w/ in other places. I'm happy to remove
> any/all which should be gone.

The two debug flags (-g, -xdebugformat) should definitely go.  I'm not sure
why the -W0 flag is there; since you don't have -xipo here, I would think
it's useless.  I could take or leave -v.  The -xcode flag is not necessary,
since you're not actually creating a shared library.  Given that, -DPIC in
CPPFLAGS shouldn't be necessary, either.

> >zoneproxy-adm/zoneproxy-adm.c:
> >
> >   - line 58: this doesn't constitute a failure?
> I don't think so? I think it means that the zoneproxy-d isn't running. I
> guess I can make it a failure. To hopefully avoid needless round trips,
> is the right thing to do to print an error then call exit (1)?

That's what I'd do.  But if it's okay that it exits 0 when the daemon isn't
running, that's okay; it just seemed wrong to me.

Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to