On Tue, Sep 24, 2024 at 04:08:11PM GMT, Markus Elfring wrote: > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > Date: Tue, 24 Sep 2024 15:55:06 +0200 > > Add jump targets so that a bit of exception handling can be better reused > at the end of this function implementation. > > Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > --- > drivers/remoteproc/qcom_q6v5_mss.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c > index 2a42215ce8e0..b398ae3083a1 100644 > --- a/drivers/remoteproc/qcom_q6v5_mss.c > +++ b/drivers/remoteproc/qcom_q6v5_mss.c > @@ -1451,9 +1451,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc) > dev_err(qproc->dev, > "failed to load segment %d from truncated file %s\n", > i, fw_name); > - ret = -EINVAL; Please keep error assignment where it is. It is much cleaner to read it this way, rather than checking the error-handling basement. > - memunmap(ptr); > - goto release_firmware; > + goto e_inval_unmap; > } > > memcpy(ptr, fw->data + phdr->p_offset, phdr->p_filesz); > @@ -1464,18 +1462,15 @@ static int q6v5_mpss_load(struct q6v5 *qproc) > ptr, phdr->p_filesz); > if (ret) { > dev_err(qproc->dev, "failed to load %s\n", fw_name); > - memunmap(ptr); > - goto release_firmware; > + goto unmap_mem; > } > > if (seg_fw->size != phdr->p_filesz) { > dev_err(qproc->dev, > "failed to load segment %d from truncated file %s\n", > i, fw_name); > - ret = -EINVAL; > release_firmware(seg_fw); > - memunmap(ptr); > - goto release_firmware; > + goto e_inval_unmap; > } > > release_firmware(seg_fw); > @@ -1528,6 +1523,12 @@ static int q6v5_mpss_load(struct q6v5 *qproc) > kfree(fw_name); > > return ret < 0 ? ret : 0; > + > +e_inval_unmap: > + ret = -EINVAL; > +unmap_mem: > + memunmap(ptr); > + goto release_firmware; Ugh. No. ptr should be a variable that is declared inside the loop. Calling memunmap outside of the for loop is incorrect. And goto just complicates things by adding non-linearity. > } > > static void qcom_q6v5_dump_segment(struct rproc *rproc, > -- > 2.46.1 > -- With best wishes Dmitry