[U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs

Steve Rae srae at broadcom.com
Fri Dec 12 19:03:01 CET 2014



On 14-11-20 02:25 AM, Linus Walleij wrote:
> There is currently a regression when using newer ARM64 compilers
> for semihosting: the way long types are inferred from context
> is no longer the same.
>
> The semihosting runtime uses long and size_t, so use this
> explicitly in the semihosting code and interface, and voila:
> the code now works again.
>
> Tested with aarch64-linux-gnu-gcc: Linaro GCC 4.9-2014.09.
>
> Cc: Darwin Rambo <drambo at broadcom.com>
> Cc: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Cc: Mark Hambleton <mark.hambleton at arm.com>
> Cc: Tom Rini <trini at ti.com>
> Suggested-by: Mark Hambleton <mark.hambleton at arm.com>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
>   arch/arm/include/asm/semihosting.h |   2 +-
>   arch/arm/lib/semihosting.c         | 101 +++++++++++++++++++------------------
>   2 files changed, 52 insertions(+), 51 deletions(-)
>
> diff --git a/arch/arm/include/asm/semihosting.h b/arch/arm/include/asm/semihosting.h
> index e59b44ed6068..835ca7e4b683 100644
> --- a/arch/arm/include/asm/semihosting.h
> +++ b/arch/arm/include/asm/semihosting.h
> @@ -12,6 +12,6 @@
>    * code for more information.
>    */
>   int smh_load(const char *fname, void *memp, int avail, int verbose);
> -int smh_len(const char *fname);
> +long smh_len(const char *fname);
>
>   #endif /* __SEMIHOSTING_H__ */
> diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c
> index 92bbabe158fe..6e1b2d182eca 100644
> --- a/arch/arm/lib/semihosting.c
> +++ b/arch/arm/lib/semihosting.c
> @@ -23,17 +23,17 @@
>   #define MODE_READ	0x0
>   #define MODE_READBIN	0x1
>
> -static int smh_read(int fd, void *memp, int len);
> -static int smh_open(const char *fname, char *modestr);
> -static int smh_close(int fd);
> -static int smh_len_fd(int fd);
> +static long smh_read(long fd, void *memp, size_t len);
> +static long smh_open(const char *fname, char *modestr);
> +static long smh_close(long fd);
> +static long smh_len_fd(long fd);
>
>   /*
>    * Call the handler
>    */
> -static int smh_trap(unsigned int sysnum, void *addr)
> +static long smh_trap(unsigned int sysnum, void *addr)
>   {
> -	register int result asm("r0");
> +	register long result asm("r0");
>   #if defined(CONFIG_ARM64)
>   	asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr));
>   #else
> @@ -51,7 +51,9 @@ static int smh_trap(unsigned int sysnum, void *addr)
>    */
>   int smh_load(const char *fname, void *memp, int avail, int verbose)
>   {
> -	int ret, fd, len;
> +	long ret;
> +	long fd;
> +	size_t len;
>
>   	ret = -1;
>
> @@ -61,21 +63,21 @@ int smh_load(const char *fname, void *memp, int avail, int verbose)
>   	/* Open the file */
>   	fd = smh_open(fname, "rb");
>   	if (fd == -1)
> -		return ret;
> +		return -1;
>
>   	/* Get the file length */
>   	ret = smh_len_fd(fd);
>   	if (ret == -1) {
>   		smh_close(fd);
> -		return ret;
> +		return -1;
>   	}
>
>   	/* Check that the file will fit in the supplied buffer */
>   	if (ret > avail) {
> -		printf("%s: ERROR ret %d, avail %u\n", __func__, ret,
> +		printf("%s: ERROR ret %ld, avail %u\n", __func__, ret,
>   		       avail);
>   		smh_close(fd);
> -		return ret;
> +		return -1;
>   	}
>
>   	len = ret;
> @@ -87,7 +89,7 @@ int smh_load(const char *fname, void *memp, int avail, int verbose)
>   		if (verbose) {
>   			printf("\n%s\n", fname);
>   			printf("    0x%8p dest\n", memp);
> -			printf("    0x%08x size\n", len);
> +			printf("    0x%08lx size\n", len);
>   			printf("    0x%08x avail\n", avail);
>   		}
>   	}
> @@ -101,54 +103,53 @@ int smh_load(const char *fname, void *memp, int avail, int verbose)
>   /*
>    * Read 'len' bytes of file into 'memp'. Returns 0 on success, else failure
>    */
> -static int smh_read(int fd, void *memp, int len)
> +static long smh_read(long fd, void *memp, size_t len)
>   {
> -	int ret;
> +	long ret;
>   	struct smh_read_s {
> -		int fd;
> +		long fd;
>   		void *memp;
> -		int len;
> +		size_t len;
>   	} read;
>
> -	debug("%s: fd %d, memp %p, len %d\n", __func__, fd, memp, len);
> +	debug("%s: fd %ld, memp %p, len %lu\n", __func__, fd, memp, len);
>
>   	read.fd = fd;
>   	read.memp = memp;
>   	read.len = len;
>
>   	ret = smh_trap(SYSREAD, &read);
> -	if (ret == 0) {
> -		return 0;
> -	} else {
> +	if (ret < 0) {
>   		/*
>   		 * The ARM handler allows for returning partial lengths,
>   		 * but in practice this never happens so rather than create
>   		 * hard to maintain partial read loops and such, just fail
>   		 * with an error message.
>   		 */
> -		printf("%s: ERROR ret %d, fd %d, len %u memp %p\n",
> +		printf("%s: ERROR ret %ld, fd %ld, len %lu memp %p\n",
>   		       __func__, ret, fd, len, memp);
> +		return -1;
>   	}
> -	return ret;
> +
> +	return 0;
>   }
>
>   /*
>    * Open a file on the host. Mode is "r" or "rb" currently. Returns a file
>    * descriptor or -1 on error.
>    */
> -static int smh_open(const char *fname, char *modestr)
> +static long smh_open(const char *fname, char *modestr)
>   {
> -	int ret, fd, mode;
> +	long fd;
> +	unsigned long mode;
>   	struct smh_open_s {
>   		const char *fname;
> -		unsigned int mode;
> -		unsigned int len;
> +		unsigned long mode;
> +		size_t len;
>   	} open;
>
>   	debug("%s: file \'%s\', mode \'%s\'\n", __func__, fname, modestr);
>
> -	ret = -1;
> -
>   	/* Check the file mode */
>   	if (!(strcmp(modestr, "r"))) {
>   		mode = MODE_READ;
> @@ -157,7 +158,7 @@ static int smh_open(const char *fname, char *modestr)
>   	} else {
>   		printf("%s: ERROR mode \'%s\' not supported\n", __func__,
>   		       modestr);
> -		return ret;
> +		return -1;
>   	}
>
>   	open.fname = fname;
> @@ -167,7 +168,7 @@ static int smh_open(const char *fname, char *modestr)
>   	/* Open the file on the host */
>   	fd = smh_trap(SYSOPEN, &open);
>   	if (fd == -1)
> -		printf("%s: ERROR fd %d for file \'%s\'\n", __func__, fd,
> +		printf("%s: ERROR fd %ld for file \'%s\'\n", __func__, fd,
>   		       fname);
>
>   	return fd;
> @@ -176,17 +177,15 @@ static int smh_open(const char *fname, char *modestr)
>   /*
>    * Close the file using the file descriptor
>    */
> -static int smh_close(int fd)
> +static long smh_close(long fd)
>   {
> -	int ret;
> -	long fdlong;
> +	long ret;
>
> -	debug("%s: fd %d\n", __func__, fd);
> +	debug("%s: fd %ld\n", __func__, fd);
>
> -	fdlong = (long)fd;
> -	ret = smh_trap(SYSCLOSE, &fdlong);
> +	ret = smh_trap(SYSCLOSE, &fd);
>   	if (ret == -1)
> -		printf("%s: ERROR fd %d\n", __func__, fd);
> +		printf("%s: ERROR fd %ld\n", __func__, fd);
>
>   	return ret;
>   }
> @@ -194,17 +193,15 @@ static int smh_close(int fd)
>   /*
>    * Get the file length from the file descriptor
>    */
> -static int smh_len_fd(int fd)
> +static long smh_len_fd(long fd)
>   {
> -	int ret;
> -	long fdlong;
> +	long ret;
>
> -	debug("%s: fd %d\n", __func__, fd);
> +	debug("%s: fd %ld\n", __func__, fd);
>
> -	fdlong = (long)fd;
> -	ret = smh_trap(SYSFLEN, &fdlong);
> +	ret = smh_trap(SYSFLEN, &fd);
>   	if (ret == -1)
> -		printf("%s: ERROR ret %d\n", __func__, ret);
> +		printf("%s: ERROR ret %ld, fd %ld\n", __func__, ret, fd);
>
>   	return ret;
>   }
> @@ -212,27 +209,31 @@ static int smh_len_fd(int fd)
>   /*
>    * Get the file length from the filename
>    */
> -int smh_len(const char *fname)
> +long smh_len(const char *fname)
>   {
> -	int ret, fd, len;
> +	long ret;
> +	long fd;
> +	long len;
>
>   	debug("%s: file \'%s\'\n", __func__, fname);
>
>   	/* Open the file */
>   	fd = smh_open(fname, "rb");
> -	if (fd == -1)
> +	if (fd < 0)
>   		return fd;
>
>   	/* Get the file length */
>   	len = smh_len_fd(fd);
> +	if (len < 0)
> +		return len;
>
>   	/* Close the file */
>   	ret = smh_close(fd);
> -	if (ret == -1)
> +	if (ret < 0)
>   		return ret;
>
> -	debug("%s: returning len %d\n", __func__, len);
> +	debug("%s: returning len %ld\n", __func__, len);
>
>   	/* Return the file length (or -1 error indication) */
> -	return len;
> +	return (long)len;

unnecessary cast ???
otherwise

Acked-by: Steve Rae <srae at broadcom.com>

>   }
>


More information about the U-Boot mailing list