This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA: Add Epiphany newlib & libgloss port


Joern Rennecke wrote:
> 3rd attempt to send this, it needs xz compression to fit... bzip2
> looked OK at 65 KB, but base64 encoding and the other parts must have pushed
> it over the limit.
>
> The binutils port went in two weeks ago, and the GCC port today,
> so this is the missing piece to compile standard C programs.
>
> 2011-11-05  Jeremy Bennett  <jeremy.bennett@embecosm.com>
> 	    Joern Rennecke  <joern.rennecke@embecosm.com>

Hi Joern,

I was curious, so did a casual review and spotted a few problems.

stat always succeeds.  Even with a very rudimentary file system, shouldn't
  stat be able to fail with ENOENT, EFAULT, ENAMETOOLONG, ENOTDIR, etc?

fstat always succeeds.  What about EBADF, EFAULT, etc.?

open.c has a bug:

    This open function can never return 0, and always sets errno to 0
    upon failure.  I'm guessing it should be "if (status < 0) ...":

        int __attribute__ ((section ("libgloss_epiphany")))
        _open (char *file,
               int   flags,
               int   mode)
        {
          int status = asm_open (file, flags, mode);
          if (!status) {
            errno = status;
            return -1;
          }
          return status;
        }	/* _open () */

link.c always fails with EMLINK;  wouldn't ENOSYS be better?
wait.c: likewise always with ECHILD;  wouldn't ENOSYS be better?
  Tools that detect lack of support recognize ENOSYS as indicating that.

read.c has a typo in its comment.  Fix it with s/Open a file/Read a file/

Using the "configure.in" name rather than "configure.ac" is really
anachronistic.  Better to switch to the latter (it's been 10 years),
if only to avoid making people wonder which version of autoconf you require.
It will probably avoid some warnings, too, for those who enable them.

It'd be nice to remove all trailing blanks.

I know most are just stubs, but it'd be nice to keep the signatures
of these functions consistent with those in libc. E.g., open's first
parameter would be "const char *" and the third parameter would have
type mode_t.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]