Discussion:
[Aarch64] Fix _lseek prototype
Christophe Lyon
2018-10-01 21:35:19 UTC
Permalink
Hi,

While building newlib for Aarch64, I noticed a warning in _lseek. This
small patch fixes the prototype.

OK?

Christophe
Eric Blake
2018-10-01 22:18:51 UTC
Permalink
Post by Christophe Lyon
Hi,
While building newlib for Aarch64, I noticed a warning in _lseek. This
small patch fixes the prototype.
+++ b/libgloss/aarch64/syscalls.c
@@ -449,6 +449,7 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
+int
_lseek (int fd, int ptr, int dir)
Why int and not off_t?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Christophe Lyon
2018-10-02 06:38:40 UTC
Permalink
Post by Eric Blake
Post by Christophe Lyon
Hi,
While building newlib for Aarch64, I noticed a warning in _lseek. This
small patch fixes the prototype.
+++ b/libgloss/aarch64/syscalls.c
@@ -449,6 +449,7 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
+int
_lseek (int fd, int ptr, int dir)
Why int and not off_t?
Because that's how the prototype is defined at the beginning of the same file.

Maybe time to revisit this?
Post by Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Christophe Lyon
2018-10-05 09:21:11 UTC
Permalink
Post by Christophe Lyon
Post by Eric Blake
Post by Christophe Lyon
Hi,
While building newlib for Aarch64, I noticed a warning in _lseek. This
small patch fixes the prototype.
+++ b/libgloss/aarch64/syscalls.c
@@ -449,6 +449,7 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
+int
_lseek (int fd, int ptr, int dir)
Why int and not off_t?
Because that's how the prototype is defined at the beginning of the same file.
Maybe time to revisit this?
Here is an updated version using "off_t" instead of "int".
OK?
Post by Christophe Lyon
Post by Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Richard Earnshaw (lists)
2018-10-05 12:25:25 UTC
Permalink
Post by Christophe Lyon
Post by Christophe Lyon
Post by Eric Blake
Post by Christophe Lyon
Hi,
While building newlib for Aarch64, I noticed a warning in _lseek. This
small patch fixes the prototype.
+++ b/libgloss/aarch64/syscalls.c
@@ -449,6 +449,7 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
+int
_lseek (int fd, int ptr, int dir)
Why int and not off_t?
Because that's how the prototype is defined at the beginning of the same file.
Maybe time to revisit this?
Here is an updated version using "off_t" instead of "int".
OK?
I think _swilseek should be fixed as well.

R.
Post by Christophe Lyon
Post by Christophe Lyon
Post by Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
newlib-4.txt
commit 93793741f0e43f091186f6ebf3690577f5388ba0
Date: Mon Oct 1 19:10:10 2018 +0000
[Aarch64] Fix _lseek prototype
* libgloss/aarch64/syscalls.c (_lseek): Fix prototype.
diff --git a/libgloss/aarch64/syscalls.c b/libgloss/aarch64/syscalls.c
index e6dd4bd..9e77230 100644
--- a/libgloss/aarch64/syscalls.c
+++ b/libgloss/aarch64/syscalls.c
@@ -66,7 +66,7 @@ int _open (const char *, int, ...);
int _swiopen (const char *, int);
int _write (int, char *, int);
int _swiwrite (int, char *, int);
-int _lseek (int, int, int);
+off_t _lseek (int, int, int);
int _swilseek (int, int, int);
int _read (int, char *, int);
int _swiread (int, char *, int);
@@ -449,6 +449,7 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
+off_t
_lseek (int fd, int ptr, int dir)
{
return _swilseek (fd, ptr, dir);
Eric Blake
2018-10-05 14:08:22 UTC
Permalink
Post by Christophe Lyon
Here is an updated version using "off_t" instead of "int".
OK?
-int _lseek (int, int, int);
+off_t _lseek (int, int, int);
Per POSIX, the primary function is off_t lseek(int, off_t, int). It
looks weird that your _lseek uses int instead of off_t offset. Is this
code only ever used on a 32-bit platform, where off_t will never be a
64-bit type? And since this is '_lseek' rather than 'lseek,' it might
be okay to have a different signature than POSIX. Even so, it's still
better to use off_t consistently, rather than in 1/2 of the places where
it is typically used.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Christophe Lyon
2018-10-08 08:54:27 UTC
Permalink
Post by Eric Blake
Post by Christophe Lyon
Here is an updated version using "off_t" instead of "int".
OK?
-int _lseek (int, int, int);
+off_t _lseek (int, int, int);
Per POSIX, the primary function is off_t lseek(int, off_t, int). It
looks weird that your _lseek uses int instead of off_t offset. Is this
code only ever used on a 32-bit platform, where off_t will never be a
64-bit type? And since this is '_lseek' rather than 'lseek,' it might
be okay to have a different signature than POSIX. Even so, it's still
better to use off_t consistently, rather than in 1/2 of the places where
it is typically used.
OK, this new patches what was recently committed to the arm port, and
adjusts several other prototypes in the same file.

Christophe
Post by Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Richard Earnshaw (lists)
2018-10-08 13:41:40 UTC
Permalink
Post by Christophe Lyon
Post by Eric Blake
Post by Christophe Lyon
Here is an updated version using "off_t" instead of "int".
OK?
-int _lseek (int, int, int);
+off_t _lseek (int, int, int);
Per POSIX, the primary function is off_t lseek(int, off_t, int). It
looks weird that your _lseek uses int instead of off_t offset. Is this
code only ever used on a 32-bit platform, where off_t will never be a
64-bit type? And since this is '_lseek' rather than 'lseek,' it might
be okay to have a different signature than POSIX. Even so, it's still
better to use off_t consistently, rather than in 1/2 of the places where
it is typically used.
OK, this new patches what was recently committed to the arm port, and
adjusts several other prototypes in the same file.
Pushed.

Thanks,

R.
Post by Christophe Lyon
Christophe
Post by Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
newlib-prototypes.txt
commit d843def543480ec873fea33ba235d309070e6eae
Date: Mon Oct 1 19:10:10 2018 +0000
[Aarch64] Syscalls: fix prototypes
This patch is similar the arm one committed recently.
* libgloss/aarch64/syscalls.c (_sbrk): Fix prototype.
Likewise.
diff --git a/libgloss/aarch64/syscalls.c b/libgloss/aarch64/syscalls.c
index e6dd4bd..7343cc6 100644
--- a/libgloss/aarch64/syscalls.c
+++ b/libgloss/aarch64/syscalls.c
@@ -57,19 +57,19 @@ int _link (void);
int _stat (const char *, struct stat *);
int _fstat (int, struct stat *);
int _swistat (int fd, struct stat * st);
-caddr_t _sbrk (int);
-int _getpid (int);
+void * _sbrk (ptrdiff_t);
+pid_t _getpid (void);
int _close (int);
clock_t _clock (void);
int _swiclose (int);
int _open (const char *, int, ...);
int _swiopen (const char *, int);
-int _write (int, char *, int);
-int _swiwrite (int, char *, int);
-int _lseek (int, int, int);
-int _swilseek (int, int, int);
-int _read (int, char *, int);
-int _swiread (int, char *, int);
+int _write (int, const char *, size_t);
+int _swiwrite (int, const char *, size_t);
+off_t _lseek (int, off_t, int);
+off_t _swilseek (int, off_t, int);
+int _read (int, void *, size_t);
+int _swiread (int, void *, size_t);
void initialise_monitor_handles (void);
static int checkerror (int);
@@ -349,7 +349,7 @@ checkerror (int result)
len, is the length in bytes to read.
Returns the number of bytes *not* written. */
int
-_swiread (int fh, char *ptr, int len)
+_swiread (int fh, void *ptr, size_t len)
{
param_block_t block[3];
@@ -364,7 +364,7 @@ _swiread (int fh, char *ptr, int len)
Translates the return of _swiread into
bytes read. */
int
-_read (int fd, char *ptr, int len)
+_read (int fd, void *ptr, size_t len)
{
int res;
struct fdent *pfd;
@@ -389,8 +389,8 @@ _read (int fd, char *ptr, int len)
}
/* fd, is a user file descriptor. */
-int
-_swilseek (int fd, int ptr, int dir)
+off_t
+_swilseek (int fd, off_t ptr, int dir)
{
int res;
struct fdent *pfd;
@@ -449,7 +449,8 @@ _swilseek (int fd, int ptr, int dir)
return -1;
}
-_lseek (int fd, int ptr, int dir)
+off_t
+_lseek (int fd, off_t ptr, int dir)
{
return _swilseek (fd, ptr, dir);
}
@@ -457,7 +458,7 @@ _lseek (int fd, int ptr, int dir)
/* fh, is a valid internal file handle.
Returns the number of bytes *not* written. */
int
-_swiwrite (int fh, char *ptr, int len)
+_swiwrite (int fh, const char *ptr, size_t len)
{
param_block_t block[3];
@@ -470,7 +471,7 @@ _swiwrite (int fh, char *ptr, int len)
/* fd, is a user file descriptor. */
int
-_write (int fd, char *ptr, int len)
+_write (int fd, const char *ptr, size_t len)
{
int res;
struct fdent *pfd;
@@ -620,7 +621,7 @@ _close (int fd)
}
int __attribute__((weak))
-_getpid (int n __attribute__ ((unused)))
+_getpid (void)
{
return 1;
}
@@ -628,8 +629,8 @@ _getpid (int n __attribute__ ((unused)))
/* Heap limit returned from SYS_HEAPINFO Angel semihost call. */
ulong __heap_limit __attribute__ ((aligned (8))) = 0xcafedead;
-caddr_t
-_sbrk (int incr)
+void *
+_sbrk (ptrdiff_t incr)
{
extern char end asm ("end"); /* Defined by the linker. */
static char *heap_end;
Loading...