Re: sysutils/xen* clang patches for review

[Joerg Sonnenberger <joerg%britannica.bec.de@localhost>]

On Fri, Mar 29, 2013 at 09:01:46AM -0400, Greg Troxel wrote:
> I know we aren't using git, but I would like to see git-style commits in
> cvs, where each 'cvs commit' invocation is a single logical change.
> That makes it far easier to understand history.   I don't mean that each
> patch should be split, but fixing upstream bugs and changing makefiles
> to add clang support seem like separate things.

Eh, no. The issues fixes are all needed to get the clang build running,
as it would otherwise require more warnings to be disabled.

> The patches are not all obvious.  I don't particularly suspect that any of
> them are incorrect, and I think odds are very good that they are all ok.
> But they do not have comments, and there are not links to the upstream
> bugtracker.  It seems like many of them should be upstream.

The majority for older release, so upstream is not likely to pick those
up.... I take it we have enough Xen developers to handle the remaining
changes.

> I am bringing up the comments/upstream-tracker-links issue because I am
> not really comfortable with pkgsrc being used to indefinitely carry
> patches that belong upstream.  While it's progress to have the patch, it
> also means that anyone doing updates has a lot more work to do.
> 
> Also, I don't think this hunk should be applied:
> 
> +-            if (write(fd, &out, sizeof(uint32_t))) ;
> ++            if (write(fd, &out, sizeof(uint32_t)))
> ++                    ;

It falls into the category of dealing with warnings. 
Compare patch-xen_arch_x86_hvm_io.c for a real bug.
The style issue is using the if in first place, but I guess glibc is to
be blamed for that.

> You didn't mention testing; have you booted the various xen kernels
> (compiled with system gcc, at least) on some combination of netbsd-5 and
> netbsd-6 and seen that dom0 and a domU are ok?  I know we don't have a
> testing policy, but xen stability seems critical and I'd like to hear
> what the people who tend to deal with xen think.

If I had a chance to test them, I wouldn't have dropped the patch set
here...

Joerg