> On Sun, Feb 15, 2015 at 08:53:08PM +0000, Miod Vallat wrote:
> > This ought to fix this problem:
>
> This looks reasonable but needs a tweak if you want to keep the existing
> behavior. In the existing code, it has this section above the copyout():
>
> rw_enter_write(&sc->sc_lock);
> if (request == SDIOCEXECMMC)
> error = sdmmc_mmc_command(sc, &cmd);
> else
> error = sdmmc_app_command(sc, &cmd);
> rw_exit(&sc->sc_lock);
> if (error && !cmd.c_error)
> cmd.c_error = error;
>
> and returns 0 even if error is non-zero. With this diff, it will either
> return the result of those functions or the result of copyout depending
> on whether ucmd->c_data is non-zero. Using the result of copyout is
> intentional, but not the result from sdmmc_{mmc,app}_command().
>
> Could you set error=0 after the above chunk or use a new variable?
> Also, one of the new lines has space indents rather than a tab.
Indeed.
Index: sdmmc.c
===================================================================
RCS file: /cvs/src/sys/dev/sdmmc/sdmmc.c,v
retrieving revision 1.36
diff -u -p -r1.36 sdmmc.c
--- sdmmc.c 1 Nov 2014 16:32:06 -0000 1.36
+++ sdmmc.c 16 Feb 2015 05:14:27 -0000
@@ -749,7 +749,7 @@ sdmmc_ioctl(struct device *self, u_long
struct sdmmc_command *ucmd;
struct sdmmc_command cmd;
void *data;
- int error;
+ int error = 0;
switch (request) {
#ifdef SDMMC_DEBUG
@@ -784,8 +784,9 @@ sdmmc_ioctl(struct device *self, u_long
M_WAITOK | M_CANFAIL);
if (data == NULL)
return ENOMEM;
- if (copyin(ucmd->c_data, data, ucmd->c_datalen))
- return EFAULT;
+ error = copyin(ucmd->c_data, data, ucmd->c_datalen);
+ if (error != 0)
+ goto exec_done;
cmd.c_data = data;
cmd.c_datalen = ucmd->c_datalen;
@@ -804,10 +805,12 @@ sdmmc_ioctl(struct device *self, u_long
ucmd->c_flags = cmd.c_flags;
ucmd->c_error = cmd.c_error;
- if (ucmd->c_data && copyout(data, ucmd->c_data,
- ucmd->c_datalen))
- return EFAULT;
+ if (ucmd->c_data)
+ error = copyout(data, ucmd->c_data, ucmd->c_datalen);
+ else
+ error = 0;
+exec_done:
if (ucmd->c_data)
free(data, M_TEMP, 0);
break;
@@ -815,7 +818,7 @@ sdmmc_ioctl(struct device *self, u_long
default:
return ENOTTY;
}
- return 0;
+ return error;
}
#endif
