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: Ping/Repost (Updated 20120127): RFA: Add Epiphany newlib & libgloss port


newlib/:

what is libc/sys/epiphany/e_printf.c all about ?  i don't see anyone else 
implementing e_printf(), and the style in this file is pretty bad.  easier to 
just punt it, and by extension, the entire libc/sys/epiphany/ subdir.

similarly, what's the point of libc/machine/epiphany/machine/stdlib.h ?  seems 
like that should just get dropped.

setjmp.S could use some touch ups -- it mixes spaces and tabs for indentation.

libgloss/:

i think you've copied way more logic than needed in your configure.in.  pretty 
sure you can (and should) drop the logic related to:
	- HAVE_GNU_LD
	- HAVE_ELF
	- HAVE_ASM_POPSECTION_DIRECTIVE
	- HAVE_SECTION_ATTRIBUTES
	- libc_symbol_prefix

similarly, i think you've copied useless stuff into your Makefile.in.  i'd drop 
the pattern rules for .c.o, .C.o, .S.o, .s.o, and .c.s.

AR_FLAGS in Makefile.in should probably be "rc" instead of "qc".

the install target in Makefile.in should have "|| exit $$?" added after the 
`install` command to catch errors.

your aclocal.m4 has copyright info, but i think that should just be an 
autogenerated file, so that doesn't make sense.

does that _exit.S need to be assembly ?  seems like it'd be trivial to merge 
it into _exit.c and then drop the .S file.

the style in all of the C files could do with cleaning up.  please review the 
GNU coding style.  i believe `indent` should help with that.

i don't think you should be adding access.c in the epiphany-specific subdir.  
this would be better implemented in common code.

why do you implement _execve() and _fork() and _getpid() and _times() and 
_wait() ?  libnosys already provides those stubs.

in epiphany-syscalls.c, i wonder why you have to hardcode each syscall.  can't 
you do something with integer constraints like:
	asm volatile ("trap %0" : : "i"(TRAP_write));
also, does your gcc port not support naming of specific registers in 
inputs/outputs ?  it's much better to write:
	asm volatile ("trap %[trapnr]"
		: "=r0"(result), "=r3"(error)
		: [trapnr] "i"(TRAP_write), "r0"(CHAN), "r1"(ADDR), "r2"(LEN));
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.


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