Xen devel - Sep 2007 - [PATCH] libfsimage: Make it build on NetBSD

Hi!

Attached patch makes libfsimage build on NetBSD
by fixing a number of those errors:

cc1: warnings being treated as errors
fsys_fat.c: In function ''fat_dir'':
fsys_fat.c:304: warning: array subscript has type ''char''

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Sep 21, 2007 at 02:39:46PM +0200, Christoph Egger wrote:
> -  if (!*dirname || isspace (*dirname))
> +  if (!*dirname || isspace ((uint8_t)*dirname))
Eww, surely not. This surely can''t give a warning on NetBSD?

john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Sep 21, 2007 at 02:39:46PM +0200, Christoph Egger
wrote:
> -  if (!*dirname || isspace (*dirname))
> +  if (!*dirname || isspace ((uint8_t)*dirname))
According to C99, the definition is "int isspace(int c)". Even char
can
be converted implicitely to int. Which problem do you intend to fix with
that? If this functions are incorrectly defined on BSD, fix that their.

Bastian

-- 
The heart is not a logical organ.
		-- Dr. Janet Wallace, "The Deadly Years", stardate 3479.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Sep 21, 2007 at 04:47:32PM +0100, John Levon
wrote:
> On Fri, Sep 21, 2007 at 02:39:46PM +0200, Christoph Egger wrote:
> > -  if (!*dirname || isspace (*dirname))
> > +  if (!*dirname || isspace ((uint8_t)*dirname))
> Eww, surely not. This surely can''t give a warning on NetBSD?
It can if it is implemented as something like:
| int char_isspace[] = {...];
| #define isspace(c) char_isspace[c]

Bastian

-- 
	"Get back to your stations!"
	"We''re beaming down to the planet, sir."
		-- Kirk and Mr. Leslie, "This Side of Paradise",
		   stardate 3417.3

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 21/9/07 13:39, "Christoph Egger" <Christoph.Egger@amd.com>
wrote:
> Attached patch makes libfsimage build on NetBSD
> by fixing a number of those errors:
> 
> cc1: warnings being treated as errors
> fsys_fat.c: In function ''fat_dir'':
> fsys_fat.c:304: warning: array subscript has type ''char''
You also cast before a comparison with a char literal (''/'').
Surely that
isn''t necessary?

For the rest, it does sound like NetBSD''s ctype.h is strictly compliant
with
ISO C, which requires a non-EOF argument to be representable as an unsigned
char. Clearly a negative argument is not representable as an unsigned char
of the same integer value, so a cast is needed to force things to comply
with the ISO spec. Bah.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 23/9/07 12:12, "Bastian Blank" <bastian@waldi.eu.org> wrote:
> On Fri, Sep 21, 2007 at 02:39:46PM +0200, Christoph Egger wrote:
>> -  if (!*dirname || isspace (*dirname))
>> +  if (!*dirname || isspace ((uint8_t)*dirname))
> 
> According to C99, the definition is "int isspace(int c)". Even
char can
> be converted implicitely to int. Which problem do you intend to fix with
> that? If this functions are incorrectly defined on BSD, fix that their.
The ISO spec is pretty unambiguous: "The c argument is an int, the value of
which the application shall ensure is a character representable as an
unsigned char or equal to the value of the macro EOF. If the argument has
any other value, the behavior is undefined."

If we sensibly take ''representable'' to mean
''representable with the same
integer value'', then the casts are clearly needed for compliance. This
interpretation even makes some sense, since it explicitly disallows
collision between character code 0xff and EOF (usually -1).

And read glibc''s ctype.h -- it goes to some effort to support
''signed char''
input, but makes it clear this is "for broken old programs".

This seems to be one of the ugly corners of the C spec where the truth is
not very palatable. :-)

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Sun, Sep 23, 2007 at 12:48:11PM +0100, Keir Fraser
wrote:
> The ISO spec is pretty unambiguous: "The c argument is an int, the
value of
> which the application shall ensure is a character representable as an
> unsigned char or equal to the value of the macro EOF. If the argument has
> any other value, the behavior is undefined."
§7.4
> If we sensibly take ''representable'' to mean
''representable with the same
> integer value'', then the casts are clearly needed for compliance.
No. ''representable'' means that the value fits in unsigned char
(0-255 on
common systems). See example 6 of §5.1.2.3 how the standard uses
representable. A cast would force that, if you use unsigned char.
> And read glibc''s ctype.h -- it goes to some effort to support
''signed char''
> input, but makes it clear this is "for broken old programs".
Yeah, it explicitely does the change which the explicite conversion to
unsigned char would do.
> This seems to be one of the ugly corners of the C spec where the truth is
> not very palatable. :-)
Haha. You want some of i + i++?

Bastian

-- 
Virtue is a relative term.
		-- Spock, "Friday''s Child", stardate 3499.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 23/9/07 13:55, "Bastian Blank" <bastian@waldi.eu.org> wrote:
>> If we sensibly take ''representable'' to mean
''representable with the same
>> integer value'', then the casts are clearly needed for
compliance.
> 
> No. ''representable'' means that the value fits in unsigned
char (0-255 on
> common systems). See example 6 of §5.1.2.3 how the standard uses
> representable. A cast would force that, if you use unsigned char.
Well, actually that''s what I meant but may have worded poorly. Yes, the
range of an unsigned char is [0, 255] and hence no negative integer can be
represented. So passing a negative char to a ctype function is invalid.
>> And read glibc''s ctype.h -- it goes to some effort to support
''signed char''
>> input, but makes it clear this is "for broken old programs".
> 
> Yeah, it explicitely does the change which the explicite conversion to
> unsigned char would do.
Actually it casts to int, at least in my installed glibc. Thus the sign is
maintained, and allowance is made by glibc for negative indexing into its
ctype array. I interpret this as a kludge to let old programs and lazy
programmers do the obvious thing in spite of the spec.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel