build fixes

build fixes

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]


Hi!

Attached patch fixes Xen build on NetBSD.

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

[-- Attachment #2: xen_tools_build.diff --]
[-- Type: text/x-diff, Size: 1249 bytes --]

diff -r a52925e1c292 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Jan 11 12:04:32 2010 +0100
+++ b/tools/libxl/libxl.h	Mon Jan 11 12:57:39 2010 +0100
@@ -20,6 +20,7 @@
 #include <netinet/in.h>
 #include <xenctrl.h>
 #include <xs.h>
+#include <sys/wait.h> /* for pid_t */
 
 typedef void (*libxl_log_callback)(void *userdata, int loglevel, const char *file,
                                    int line, const char *func, char *s);
diff -r a52925e1c292 tools/libxl/xl.c
--- a/tools/libxl/xl.c	Mon Jan 11 12:04:32 2010 +0100
+++ b/tools/libxl/xl.c	Mon Jan 11 12:57:39 2010 +0100
@@ -53,7 +53,7 @@ static int domain_qualifier_to_domid(str
 
     alldigit = 1;
     for (i = 0; p[i]; i++) {
-        if (!isdigit(p[i])) {
+        if (!isdigit((uint8_t)p[i])) {
             alldigit = 0;
             break;
         }
diff -r a52925e1c292 tools/xenpaging/file_ops.c
--- a/tools/xenpaging/file_ops.c	Mon Jan 11 12:04:32 2010 +0100
+++ b/tools/xenpaging/file_ops.c	Mon Jan 11 12:57:39 2010 +0100
@@ -36,7 +36,7 @@ static int file_op(int fd, void *page, i
     int bytes;
     int ret;
 
-    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
+    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
 
     total = 0;
     while ( total < PAGE_SIZE )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: build fixes

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] build fixes"):
> Attached patch fixes Xen build on NetBSD.

Nack.

> +#include <sys/wait.h> /* for pid_t */

This is fine but should read:
   #include <sys/wait.h>
as there is no point putting
   /* for somethingorother */
after every #include.

> -        if (!isdigit(p[i])) {
> +        if (!isdigit((uint8_t)p[i])) {

This is correct on all platforms.  But it should be fixed by some more
general approach, as it's needed for almost every call to a ctype
macro.

> -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
> +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);

This is just wrong.  It will break on >2G files.

Ian.

Re: build fixes

[Christoph Egger]

On Tuesday 12 January 2010 18:22:01 Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] build fixes"):
> > Attached patch fixes Xen build on NetBSD.
>
> Nack.

?

> > +#include <sys/wait.h> /* for pid_t */
>
> This is fine but should read:
>    #include <sys/wait.h>
> as there is no point putting
>    /* for somethingorother */
> after every #include.
>
> > -        if (!isdigit(p[i])) {
> > +        if (!isdigit((uint8_t)p[i])) {
>
> This is correct on all platforms.  But it should be fixed by some more
> general approach, as it's needed for almost every call to a ctype
> macro.

Any suggestions ?

> > -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
> > +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
>
> This is just wrong.  It will break on >2G files.

No, lseek64() doesn't exist. lseek() works fine on files > 2GB on NetBSD.
I think, on Linux you need to define LARGEFILES or something like that.

Christoph


-- 
---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

Re: build fixes

[Keir Fraser]

On 12/01/2010 17:22, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

>> -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
>> +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
> 
> This is just wrong.  It will break on >2G files.

We build everything under tools/ with LFS support enabled, so off_t is 64
bits even in 32-bit environments. FYI The bits to enable LFS support are in
tools/Rules.mk.

 -- Keir

Re: build fixes

[Patrick Colp]

Keir Fraser wrote:
> On 12/01/2010 17:22, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:
> 
>>> -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
>>> +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
>> This is just wrong.  It will break on >2G files.
> 
> We build everything under tools/ with LFS support enabled, so off_t is 64
> bits even in 32-bit environments. FYI The bits to enable LFS support are in
> tools/Rules.mk.

According to the lseek64 man page, for 32-bit architectures to have 64-bit 
off_t, you need:

#define _FILE_OFFSET_BITS 64

Which, as far as I know, isn't set. LFS just makes lseek64 available 
(without it, the function doesn't exist).


Patrick

Re: build fixes

[Christoph Egger]

On Tuesday 12 January 2010 18:42:50 Patrick Colp wrote:
> Keir Fraser wrote:
> > On 12/01/2010 17:22, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:
> >>> -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
> >>> +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
> >>
> >> This is just wrong.  It will break on >2G files.
> >
> > We build everything under tools/ with LFS support enabled, so off_t is 64
> > bits even in 32-bit environments. FYI The bits to enable LFS support are
> > in tools/Rules.mk.
>
> According to the lseek64 man page, for 32-bit architectures to have 64-bit
> off_t, you need:
>
> #define _FILE_OFFSET_BITS 64
>
> Which, as far as I know, isn't set. LFS just makes lseek64 available
> (without it, the function doesn't exist).

Ah, good to know. Since lseek64() isn't POSIX and therefore
breaks build on non-Linux, LFS should be disabled and
-D_FILE_OFFSET_BITS=64 should be used in CFLAGS under tools/ .

Christoph


-- 
---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

Re: build fixes

[Ian Jackson]

Keir Fraser writes ("Re: [Xen-devel] build fixes"):
> On 12/01/2010 17:22, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:
> >> -    seek_ret = lseek64(fd, i << PAGE_SHIFT, SEEK_SET);
> >> +    seek_ret = lseek(fd, i << PAGE_SHIFT, SEEK_SET);
> > 
> > This is just wrong.  It will break on >2G files.
> 
> We build everything under tools/ with LFS support enabled, so off_t is 64
> bits even in 32-bit environments. FYI The bits to enable LFS support are in
> tools/Rules.mk.

Oh, good, sorry for not spotting that.

Ian.

Re: build fixes

[Keir Fraser]

On 12/01/2010 17:42, "Patrick Colp" <pjcolp@cs.ubc.ca> wrote:

>> We build everything under tools/ with LFS support enabled, so off_t is 64
>> bits even in 32-bit environments. FYI The bits to enable LFS support are in
>> tools/Rules.mk.
> 
> According to the lseek64 man page, for 32-bit architectures to have 64-bit
> off_t, you need:
> 
> #define _FILE_OFFSET_BITS 64
> 
> Which, as far as I know, isn't set. LFS just makes lseek64 available
> (without it, the function doesn't exist).

I don't think so. 'getconf LFS_CFLAGS' returns -D_FILE_OFFSET_BITS=64 on a
32-bit host. It's LFS64_CFLAGS that gets you the explicit model for
large-file access (i.e., where you have to explicitly use the 64-bit off_t
and lseek variants).

 -- Keir