[PATCH 2/8] semihosting: Add support for writing to a file

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 4/30/20 7:36 PM, Sughosh Ganu wrote:
> Add a function to enable writing to a file. Currently, support is
> added for writing to a binary file. This would be used for
> implementing the firmware update functionality for the qemu arm64
> platform.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>  arch/arm/lib/semihosting.c | 41 ++++++++++++++++++++++++++++++++++++--
>  include/semihosting.h      |  1 +
>  2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c
> index 3aeda1303a..08181132d1 100644
> --- a/arch/arm/lib/semihosting.c
> +++ b/arch/arm/lib/semihosting.c
> @@ -17,11 +17,18 @@
>
>  #define SYSOPEN		0x01
>  #define SYSCLOSE	0x02
> +#define SYSWRITE	0x5
>  #define SYSREAD		0x06
>  #define SYSFLEN		0x0C
>
> -#define MODE_READ	0x0
> -#define MODE_READBIN	0x1
> +#define MODE_READ		0x0
> +#define MODE_READBIN		0x1
> +#define MODE_READPLUS		0x2
> +#define MODE_READPLUSBIN	0x3
> +#define MODE_WRITE		0x4
> +#define MODE_WRITEBIN		0x5
> +#define MODE_WRITEPLUS		0x6
> +#define MODE_WRITEPLUSBIN	0x7
>
>  /*
>   * Call the handler
> @@ -61,6 +68,8 @@ long smh_open(const char *fname, char *modestr)
>  		mode = MODE_READ;
>  	} else if (!(strcmp(modestr, "rb"))) {
>  		mode = MODE_READBIN;
> +	} else if (!strcmp(modestr, "w+b")) {
> +		mode = MODE_WRITEPLUSBIN;

We could have a string matching each of the 8 constants above:

static char modes[] = "r\0\0\0rb\0\0r+\0\0r+b\0w\0\0\0wb\0\0w+\0\0w+b";
// This should use less bytes than 8 separate strings.

        for (mode = 0; mode < 8; ++mode) {
                if (!strcmp(&modes[4 * mode], modestr))
                        break;
        }
        if (mode & 8)
		printf("%s: ERROR mode \'%s\' not supported\n" ...

But why are we passing the mode as string anyway?
We should use an enum parameter instead.

Best regards

Heinrich

>  	} else {
>  		printf("%s: ERROR mode \'%s\' not supported\n", __func__,
>  		       modestr);
> @@ -114,6 +123,34 @@ long smh_read(long fd, void *memp, size_t len)
>  	return 0;
>  }
>
> +/*
> + * Write 'len' bytes into the file referenced by the fd. Returns 0 on success, else
> + * a negavite value for failure
> + */
> +long smh_write(long fd, void *memp, size_t len)
> +{
> +	long ret;
> +	struct smh_write_s {
> +		long fd;
> +		void *memp;
> +		size_t len;
> +	} write;
> +
> +	debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len);
> +
> +	write.fd = fd;
> +	write.memp = memp;
> +	write.len = len;
> +
> +	ret = smh_trap(SYSWRITE, &write);
> +
> +	if (ret > 0)
> +		printf("%s: ERROR ret %ld, fd %ld, len %zu, memp %p\n",
> +		       __func__, ret, fd, len, memp);
> +
> +	return ret == 0 ? 0 : -1;
> +}
> +
>  /*
>   * Close the file using the file descriptor
>   */
> diff --git a/include/semihosting.h b/include/semihosting.h
> index f1bf419275..fa5cecddf2 100644
> --- a/include/semihosting.h
> +++ b/include/semihosting.h
> @@ -8,6 +8,7 @@
>
>  long smh_open(const char *fname, char *modestr);
>  long smh_read(long fd, void *memp, size_t len);
> +long smh_write(long fd, void *memp, size_t len);
>  long smh_close(long fd);
>
>  #endif /* _SEMIHOSTING_H_ */
>