Hi! Attached patch makes libfsimage build again on NetBSD. uchar_t is not defined because both FSYS_ZFS and FSIMAGE are defined at build time. Also fix warnings with ctype. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger writes ("[Xen-devel] [PATCH] libfsimage: zfs build fix"):> Attached patch makes libfsimage build again on NetBSD. > > uchar_t is not defined because both FSYS_ZFS and FSIMAGE > are defined at build time. > Also fix warnings with ctype.The ctype problems are more than warnings - they would give undefined behaviour if high-bit-set characters are passed.> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Thu, Apr 15, 2010 at 10:10 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> Christoph Egger writes ("[Xen-devel] [PATCH] libfsimage: zfs build fix"): >> Attached patch makes libfsimage build again on NetBSD.The fsimage zfs patch was changed around to make it simpler to keep in sync with the grub changes. fsys_zfs.c, etc are exact copies of what''s in grub e.g. http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/grub/grub-0.97/stage2/fsys_zfs.c fsi_zfs.* are kludges to get the grub code to fit into libfsimage and is the only code which is different from the grub code. It would be easier to keep in sync if this would be continued.>> uchar_t is not defined because both FSYS_ZFS and FSIMAGE >> are defined at build time.for this, I would suggest extending this in fsi_zfs.h #ifdef __linux__ typedef unsigned char uchar_t; #endif>> Also fix warnings with ctype. > > The ctype problems are more than warnings - they would give undefined > behaviour if high-bit-set characters are passed.I didn''t write that code and agree that it''s pretty ugly :-). I assume you talking about where the char is sign extended or not? Just curious, what''s the case that isspace() gives a wrong answer here? MRJ>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 15/04/2010 16:51, "Mark Johnson" <johnson.nh@gmail.com> wrote:> On Thu, Apr 15, 2010 at 10:10 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> > wrote: >> Christoph Egger writes ("[Xen-devel] [PATCH] libfsimage: zfs build fix"): >>> Attached patch makes libfsimage build again on NetBSD. > > The fsimage zfs patch was changed around to make it simpler > to keep in sync with the grub changes. fsys_zfs.c, etc are exact copies > of what''s in grub e.g. > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/grub/grub-0.97/s > tage2/fsys_zfs.c > > fsi_zfs.* are kludges to get the grub code to fit into libfsimage and > is the only code which is different from the grub code. > > It would be easier to keep in sync if this would be continued.Unfortunately with -Wall the zfs sources spewed a quite unreasonable number of warnings. Enough for me to get annoyed, go clean them all up, and add -Werror. I don''t know how important it is to keep the files identical? It''s not like interfaces or anything have been changed: all the modifications are trivial one-liners. Basically, it should still be possible to dump a new version of the zfs code into our tree sometime down the line, and then go xfix up the build warnings again (it didn''t take that long). Doesn''t that suffice? You could even try getting GRUB guys to accept our build cleanups upstream (fat chance given my previous experiences with them, I''ll admit). ;-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mark Johnson writes ("Re: [Xen-devel] [PATCH] libfsimage: zfs build fix"):> On Thu, Apr 15, 2010 at 10:10 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > > The ctype problems are more than warnings - they would give undefined > > behaviour if high-bit-set characters are passed. > > I didn''t write that code and agree that it''s pretty ugly :-). > I assume you talking about where the char is sign extended > or not? Just curious, what''s the case that isspace() gives > a wrong answer here?Yes. If you have high bit set characters then chars are negative. isspace et al take an int which was converted from an unsigned char, and it is not permitted to pass negative numbers to them (except EOF). So char *some_string; ... if (isspace(some_string[14])) {... is wrong because if some_string[14] is a character >127 (say, 169 which is ISO-LATIN-15 for COPYRIGHT SIGH), then isspace is passed a negative integer (-87). If you pass a negative number (other than EOF) to isspace, your program may crash or access random data. Therefore EVERY call to isspace needs to be written like this if (isspace((unsigned char)some_string[14])) {... This is of course a design error in the C library. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel