[Aarch64] Fix _lseek prototype

Message ID CAKdteObmxuMd0_m-o4f3-jWwy4+u=px_FUCRHZyE-FLwtVfMCg@mail.gmail.com
State New
Headers show
Series
  • [Aarch64] Fix _lseek prototype
Related show

Commit Message

Christophe Lyon Oct. 1, 2018, 9:35 p.m.
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.

Comments

Eric Blake Oct. 1, 2018, 10:18 p.m. | #1
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
Christophe Lyon Oct. 2, 2018, 6:38 a.m. | #2
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
Christophe Lyon Oct. 5, 2018, 9:21 a.m. | #3
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);
Richard Earnshaw (lists) Oct. 5, 2018, 12:25 p.m. | #4
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);
Eric Blake Oct. 5, 2018, 2:08 p.m. | #5
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
Christophe Lyon Oct. 8, 2018, 8:54 a.m. | #6
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;
Richard Earnshaw (lists) Oct. 8, 2018, 1:41 p.m. | #7
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;

Patch

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