| Message ID | CAKdteObmxuMd0_m-o4f3-jWwy4+u=px_FUCRHZyE-FLwtVfMCg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Aarch64] Fix _lseek prototype | expand |
On 10/1/18 4:35 PM, Christophe Lyon wrote: > 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
On Tue, 2 Oct 2018 at 00:18, Eric Blake <eblake@redhat.com> wrote: > > On 10/1/18 4:35 PM, Christophe Lyon wrote: > > 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? > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org
On Tue, 2 Oct 2018 at 08:38, Christophe Lyon <christophe.lyon@linaro.org> wrote: > > On Tue, 2 Oct 2018 at 00:18, Eric Blake <eblake@redhat.com> wrote: > > > > On 10/1/18 4:35 PM, Christophe Lyon wrote: > > > 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? > > -- > > Eric Blake, Principal Software Engineer > > Red Hat, Inc. +1-919-301-3266 > > Virtualization: qemu.org | libvirt.org commit 93793741f0e43f091186f6ebf3690577f5388ba0 Author: Christophe Lyon <christophe.lyon@linaro.org> Date: Mon Oct 1 19:10:10 2018 +0000 [Aarch64] Fix _lseek prototype 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> * 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);
On 05/10/18 10:21, Christophe Lyon wrote: > On Tue, 2 Oct 2018 at 08:38, Christophe Lyon <christophe.lyon@linaro.org> wrote: >> >> On Tue, 2 Oct 2018 at 00:18, Eric Blake <eblake@redhat.com> wrote: >>> >>> On 10/1/18 4:35 PM, Christophe Lyon wrote: >>>> 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. > >>> -- >>> Eric Blake, Principal Software Engineer >>> Red Hat, Inc. +1-919-301-3266 >>> Virtualization: qemu.org | libvirt.org >>> >>> newlib-4.txt >>> >>> >>> commit 93793741f0e43f091186f6ebf3690577f5388ba0 >>> Author: Christophe Lyon <christophe.lyon@linaro.org> >>> Date: Mon Oct 1 19:10:10 2018 +0000 >>> >>> [Aarch64] Fix _lseek prototype >>> >>> 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> >>> >>> * 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);
On 10/5/18 4:21 AM, Christophe Lyon wrote: > 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
On Fri, 5 Oct 2018 at 16:08, Eric Blake <eblake@redhat.com> wrote: > > On 10/5/18 4:21 AM, Christophe Lyon wrote: > > 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 > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org commit d843def543480ec873fea33ba235d309070e6eae Author: Christophe Lyon <christophe.lyon@linaro.org> Date: Mon Oct 1 19:10:10 2018 +0000 [Aarch64] Syscalls: fix prototypes This patch is similar the arm one committed recently. 2018-10-08 Christophe Lyon <christophe.lyon@linaro.org> * libgloss/aarch64/syscalls.c (_sbrk): Fix prototype. (_getpid, _write, _swiwrite, _lseek, _swilseek, _read, _wriread): 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;
On 08/10/18 09:54, Christophe Lyon wrote: > On Fri, 5 Oct 2018 at 16:08, Eric Blake <eblake@redhat.com> wrote: >> >> On 10/5/18 4:21 AM, Christophe Lyon wrote: >>> 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. > Christophe > >> -- >> Eric Blake, Principal Software Engineer >> Red Hat, Inc. +1-919-301-3266 >> Virtualization: qemu.org | libvirt.org >> >> newlib-prototypes.txt >> >> >> commit d843def543480ec873fea33ba235d309070e6eae >> Author: Christophe Lyon <christophe.lyon@linaro.org> >> Date: Mon Oct 1 19:10:10 2018 +0000 >> >> [Aarch64] Syscalls: fix prototypes >> >> This patch is similar the arm one committed recently. >> >> 2018-10-08 Christophe Lyon <christophe.lyon@linaro.org> >> >> * libgloss/aarch64/syscalls.c (_sbrk): Fix prototype. >> (_getpid, _write, _swiwrite, _lseek, _swilseek, _read, _wriread): >> 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;
diff --git a/libgloss/aarch64/syscalls.c b/libgloss/aarch64/syscalls.c index e6dd4bd..d6022fc 100644 --- a/libgloss/aarch64/syscalls.c +++ 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) { return _swilseek (fd, ptr, dir);
Hi, While building newlib for Aarch64, I noticed a warning in _lseek. This small patch fixes the prototype. OK? Christophe commit 21997b227745a47a9257096c5cd172e17eaa823b Author: Christophe Lyon <christophe.lyon@linaro.org> Date: Mon Oct 1 19:10:10 2018 +0000 [Aarch64] Fix _lseek prototype 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> * libgloss/aarch64/syscalls.c (_lseek): Fix prototype.