The "ret" variable needs to be signed for the error handling to work.
Fixes: 4cbe60cf5ad6 ("soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)")
Signed-off-by: Dan Carpenter <[email protected]>
---
drivers/soc/qcom/dcc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
index d4101f79cb5d..1e2cbefc1655 100644
--- a/drivers/soc/qcom/dcc.c
+++ b/drivers/soc/qcom/dcc.c
@@ -808,8 +808,9 @@ static ssize_t config_reset_write(struct file *filp,
const char __user *user_buf, size_t count,
loff_t *ppos)
{
- unsigned int val, ret;
+ unsigned int val;
struct dcc_drvdata *drvdata = filp->private_data;
+ int ret;
ret = kstrtouint_from_user(user_buf, count, 0, &val);
if (ret < 0)
--
2.35.1
It feels very wrong to assign "cfg.sram_offset" to be out of bounds.
Fortunately, this is just dead code and can be deleted.
Signed-off-by: Dan Carpenter <[email protected]>
---
drivers/soc/qcom/dcc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
index 1e2cbefc1655..681d55018e66 100644
--- a/drivers/soc/qcom/dcc.c
+++ b/drivers/soc/qcom/dcc.c
@@ -483,10 +483,8 @@ static int dcc_emit_config(struct dcc_drvdata *drvdata, unsigned int curr_list)
/* Update ram_cfg and check if the data will overstep */
drvdata->ram_cfg = (cfg.sram_offset + total_len) / 4;
- if (cfg.sram_offset + total_len > drvdata->ram_size) {
- cfg.sram_offset += total_len;
+ if (cfg.sram_offset + total_len > drvdata->ram_size)
goto overstep;
- }
drvdata->ram_start = cfg.sram_offset / 4;
return 0;
--
2.35.1
The commit message should be written in an imperative manner,
such as "Fix X, make Y do Z"
For the code:
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 5.01.2023 11:53, Dan Carpenter wrote:
> The "ret" variable needs to be signed for the error handling to work.
>
> Fixes: 4cbe60cf5ad6 ("soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)")
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> drivers/soc/qcom/dcc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
> index d4101f79cb5d..1e2cbefc1655 100644
> --- a/drivers/soc/qcom/dcc.c
> +++ b/drivers/soc/qcom/dcc.c
> @@ -808,8 +808,9 @@ static ssize_t config_reset_write(struct file *filp,
> const char __user *user_buf, size_t count,
> loff_t *ppos)
> {
> - unsigned int val, ret;
> + unsigned int val;
> struct dcc_drvdata *drvdata = filp->private_data;
> + int ret;
>
> ret = kstrtouint_from_user(user_buf, count, 0, &val);
> if (ret < 0)
On Thu, Jan 05, 2023 at 12:47:45PM +0100, Konrad Dybcio wrote:
> The commit message should be written in an imperative manner,
> such as "Fix X, make Y do Z"
>
Imperative tense requirements are a symptom of bureaucracy run mad. I
always want to push back against that. We accrete layers of
requirements like case-bearing leaf beetles making armor out of poop.
regards,
dan carpenter
On 1/5/23 4:53 AM, Dan Carpenter wrote:
> The "ret" variable needs to be signed for the error handling to work.
The fix looks good. I'm sorry I missed this one in review.
Reviewed-by: Alex Elder <[email protected]>
>
> Fixes: 4cbe60cf5ad6 ("soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)")
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> drivers/soc/qcom/dcc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
> index d4101f79cb5d..1e2cbefc1655 100644
> --- a/drivers/soc/qcom/dcc.c
> +++ b/drivers/soc/qcom/dcc.c
> @@ -808,8 +808,9 @@ static ssize_t config_reset_write(struct file *filp,
> const char __user *user_buf, size_t count,
> loff_t *ppos)
> {
> - unsigned int val, ret;
> + unsigned int val;
> struct dcc_drvdata *drvdata = filp->private_data;
> + int ret;
>
> ret = kstrtouint_from_user(user_buf, count, 0, &val);
> if (ret < 0)