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: [PATCH, NDS32] patch for nds32 port


Hi Pei-Shiang Hung,


On Dec 11 18:30, Pei-Shiang Hung wrote:
> Dear all,
> 
> Total are 3 patches.
> The first patch doesn't include the generated files.

Thanks.  I scanned the patches and patch 1 and 3 look good to me.
In patch 2 I see a few (minor) issues.

>  * libgloss/nds32/_exit.S : NDS virtual hosting support
>  * libgloss/nds32/_getpid.S : NDS virtual hosting support
>  * libgloss/nds32/_gettimeofday.S : NDS virtual hosting support
>  * libgloss/nds32/_isatty.S : NDS virtual hosting support
>  * libgloss/nds32/_kill.S : NDS virtual hosting support
>  * libgloss/nds32/_link.S : NDS virtual hosting support
>  * libgloss/nds32/_times.S : NDS virtual hosting support
>  * libgloss/nds32/_unlink.S : NDS virtual hosting support

I think this entry is not right.  Your unlink.S patch fixes a copy/paste
bug only AFAICS.

> +	.size   _getpid, .-_getpid
> diff --git a/libgloss/nds32/_gettimeofday.S b/libgloss/nds32/_gettimeofday.S
> index adc5f68..e78bd5e 100644
> --- a/libgloss/nds32/_gettimeofday.S
> +++ b/libgloss/nds32/_gettimeofday.S
> @@ -30,13 +30,19 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  #ifdef __NDS32_VH__
>  
>  #include "vh.h"
> -.extern _impure_ptr
>  TYPE3 _gettimeofday, VH_GETTIMEOFDAY
>  
>  #else	/* not __NDS32_VH__ */
>  
>  #include "../syscall.h"
>  #include "syscall_extra.h"
> -SYS_WRAPPER _gettimeofday, SYS_gettimeofday
> +	.text
> +	.global	_gettimeofday
> +	.type	_gettimeofday, @function
> +	.align	2
> +_gettimeofday:
> +	syscall	SYS_gettimeofday
> +	ret
> +	.size   _gettimeofday, .-_gettimeofday

So, what's the difference to the old implementation?  AFAICS, the
new implementation neglects to call __syscall_error_handler, but
otherwise it's the same.  What's the advantage?  In how far does
this change add "NDS virtual hosting support"?  Care to explain?

> @@ -97,22 +128,54 @@ _start:
>  	 */
>  	la	$r0, _edata
>  	la	$r1, _end
> +	/* !!! WATCH OUT !!!  */

Watch out?  Yes, ok, but... what for?  Either this comment should go away,
or it would help to extend the comment to explain what for and why.

Under the premise that I'm not at all familiar with the CPU, the rest
looks ok to me.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgppkz6rLZzl0.pgp
Description: PGP signature


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