| Message ID | CD4FEEA0-A417-4424-8677-3B708483A109@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
cc'ing Richard. On Thu, Jun 4, 2020 at 5:53 AM Joe Ramsay <Joe.Ramsay@arm.com> wrote: > Hi! > > Previously, the implementation of _swistat() for ARM assumed all files > to be of type character device. This change adds support for stat-ing > regular files. Tested with arm-none-eabi > --- > libgloss/arm/syscalls.c | 43 +++++++++++++++++++++++++++++-------------- > 1 file changed, 29 insertions(+), 14 deletions(-) > > diff --git a/libgloss/arm/syscalls.c b/libgloss/arm/syscalls.c > index fc394f9..b4bd329 100644 > --- a/libgloss/arm/syscalls.c > +++ b/libgloss/arm/syscalls.c > @@ -728,33 +728,48 @@ _sbrk (ptrdiff_t incr) > int > _swistat (int fd, struct stat * st) > { > - struct fdent *pfd; > - int res; > - > - pfd = findslot (fd); > + struct fdent *pfd = findslot (fd); > if (pfd == NULL) > { > errno = EBADF; > return -1; > } > > - /* Always assume a character device, > - with 1024 byte blocks. */ > - st->st_mode |= S_IFCHR; > - st->st_blksize = 1024; > + int isCharDevice, flen; > #ifdef ARM_RDI_MONITOR > - res = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); > + isCharDevice = checkerror (do_AngelSWI (AngelSWI_Reason_IsTTY, > &pfd->handle)); > + flen = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); > #else > asm ("mov r0, %2; swi %a1; mov %0, r0" > - : "=r" (res) > + : "=r" (isCharDevice) > + : "i" (SWI_IsTTY), "r" (pfd->handle) > + : "r0"); > + checkerror(isCharDevice); > + asm ("mov r0, %2; swi %a1; mov %0, r0" > + : "=r" (flen) > : "i" (SWI_Flen), "r" (pfd->handle) > : "r0"); > - checkerror (res); > + checkerror(flen); > #endif > - if (res == -1) > + > + if (isCharDevice != 0 && isCharDevice != 1) > + return error(-1); > + > + if (flen == -1) > return -1; > - /* Return the file size. */ > - st->st_size = res; > + > + st->st_size = flen; > + /* Always assume 1024 byte blocks */ > + st->st_blksize = 1024; > + > + if (isCharDevice) > + st->st_mode |= S_IFCHR; > + else > + { > + st->st_mode |= S_IFREG; > + st->st_blocks = (flen + 1023) / 1024; > + } > + > return 0; > } > > -- > 2.7.4 > >
On 04/06/2020 10:53, Joe Ramsay wrote: > Hi! > > Previously, the implementation of _swistat() for ARM assumed all files > to be of type character device. This change adds support for stat-ing > regular files. Tested with arm-none-eabi Firstly, there's a second version of this function, in newlib/sysdeps/arm; the two really need to be kept in sync. Unfortunately, there have been other cases where this hasn't happened, so things are getting a bit messy now.... > --- > libgloss/arm/syscalls.c | 43 +++++++++++++++++++++++++++++-------------- > 1 file changed, 29 insertions(+), 14 deletions(-) > > diff --git a/libgloss/arm/syscalls.c b/libgloss/arm/syscalls.c > index fc394f9..b4bd329 100644 > --- a/libgloss/arm/syscalls.c > +++ b/libgloss/arm/syscalls.c > @@ -728,33 +728,48 @@ _sbrk (ptrdiff_t incr) > int > _swistat (int fd, struct stat * st) > { > - struct fdent *pfd; > - int res; > - > - pfd = findslot (fd); > + struct fdent *pfd = findslot (fd); > if (pfd == NULL) > { > errno = EBADF; > return -1; > } > > - /* Always assume a character device, > - with 1024 byte blocks. */ > - st->st_mode |= S_IFCHR; > - st->st_blksize = 1024; > + int isCharDevice, flen; > #ifdef ARM_RDI_MONITOR > - res = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); > + isCharDevice = checkerror (do_AngelSWI (AngelSWI_Reason_IsTTY, &pfd->handle)); > + flen = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); > #else > asm ("mov r0, %2; swi %a1; mov %0, r0" > - : "=r" (res) > + : "=r" (isCharDevice) > + : "i" (SWI_IsTTY), "r" (pfd->handle) > + : "r0"); > + checkerror(isCharDevice); > + asm ("mov r0, %2; swi %a1; mov %0, r0" > + : "=r" (flen) > : "i" (SWI_Flen), "r" (pfd->handle) > : "r0"); > - checkerror (res); > + checkerror(flen); > #endif I don't think you should make the second SWI call if the first one failed, so you should bail out immediately on failure. Also, checkerror already calls error, so it is redundant to call it a second time. > - if (res == -1) > + > + if (isCharDevice != 0 && isCharDevice != 1) > + return error(-1); I'm not sure the logic of that is right, even though in practice the only possible results are 0, 1 and -1. error() should only be called if isCharDevice is -1, since otherwise the errno will be set to something meaningless, but see above, this needs to be done earlier. > + > + if (flen == -1) > return -1; > - /* Return the file size. */ > - st->st_size = res; > + > + st->st_size = flen; > + /* Always assume 1024 byte blocks */ > + st->st_blksize = 1024; > + > + if (isCharDevice) > + st->st_mode |= S_IFCHR; > + else > + { > + st->st_mode |= S_IFREG; > + st->st_blocks = (flen + 1023) / 1024; > + } Try to avoid manifest constants in the code. Use a #define and write expressions in terms of that. R. > + > return 0; > } > > -- > 2.7.4 >
diff --git a/libgloss/arm/syscalls.c b/libgloss/arm/syscalls.c index fc394f9..b4bd329 100644 --- a/libgloss/arm/syscalls.c +++ b/libgloss/arm/syscalls.c @@ -728,33 +728,48 @@ _sbrk (ptrdiff_t incr) int _swistat (int fd, struct stat * st) { - struct fdent *pfd; - int res; - - pfd = findslot (fd); + struct fdent *pfd = findslot (fd); if (pfd == NULL) { errno = EBADF; return -1; } - /* Always assume a character device, - with 1024 byte blocks. */ - st->st_mode |= S_IFCHR; - st->st_blksize = 1024; + int isCharDevice, flen; #ifdef ARM_RDI_MONITOR - res = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); + isCharDevice = checkerror (do_AngelSWI (AngelSWI_Reason_IsTTY, &pfd->handle)); + flen = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle)); #else asm ("mov r0, %2; swi %a1; mov %0, r0" - : "=r" (res) + : "=r" (isCharDevice) + : "i" (SWI_IsTTY), "r" (pfd->handle) + : "r0"); + checkerror(isCharDevice); + asm ("mov r0, %2; swi %a1; mov %0, r0" + : "=r" (flen) : "i" (SWI_Flen), "r" (pfd->handle) : "r0"); - checkerror (res); + checkerror(flen); #endif - if (res == -1) + + if (isCharDevice != 0 && isCharDevice != 1) + return error(-1); + + if (flen == -1) return -1; - /* Return the file size. */ - st->st_size = res; + + st->st_size = flen; + /* Always assume 1024 byte blocks */ + st->st_blksize = 1024; + + if (isCharDevice) + st->st_mode |= S_IFCHR; + else + { + st->st_mode |= S_IFREG; + st->st_blocks = (flen + 1023) / 1024; + } + return 0; }