* [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config()
@ 2022-11-25 12:03 Wang Yufen
2022-11-25 12:03 ` [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options() Wang Yufen
2022-11-25 14:31 ` [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Andy Shevchenko
0 siblings, 2 replies; 5+ messages in thread
From: Wang Yufen @ 2022-11-25 12:03 UTC (permalink / raw)
To: bvanassche, jgg, leon, dennis.dalessandro
Cc: linux-rdma, linux-kernel, andriy.shevchenko, bart.vanassche,
easwar.hariharan, Wang Yufen
In the previous while loop, "ret" may be assigned zero. Therefore,
"ret" needs to be assigned -EINVAL at the beginning of each loop.
Fixes: 97167e813415 ("staging/rdma/hfi1: Tune for unknown channel if configuration file is absent")
Signed-off-by: Wang Yufen <wangyufen@huawei.com>
---
drivers/infiniband/hw/hfi1/firmware.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/hw/hfi1/firmware.c b/drivers/infiniband/hw/hfi1/firmware.c
index 1d77514..c179dfe 100644
--- a/drivers/infiniband/hw/hfi1/firmware.c
+++ b/drivers/infiniband/hw/hfi1/firmware.c
@@ -1788,6 +1788,7 @@ int parse_platform_config(struct hfi1_devdata *dd)
* being used.
*/
while (ptr < (u32 *)(dd->platform_config.data + file_length)) {
+ ret = -EINVAL;
header1 = *ptr;
header2 = *(ptr + 1);
if (header1 != ~header2) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options()
2022-11-25 12:03 [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Wang Yufen
@ 2022-11-25 12:03 ` Wang Yufen
2022-11-25 14:37 ` Andy Shevchenko
2022-11-25 14:31 ` [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Andy Shevchenko
1 sibling, 1 reply; 5+ messages in thread
From: Wang Yufen @ 2022-11-25 12:03 UTC (permalink / raw)
To: bvanassche, jgg, leon, dennis.dalessandro
Cc: linux-rdma, linux-kernel, andriy.shevchenko, bart.vanassche,
easwar.hariharan, Wang Yufen
In the previous while loop, "ret" may be assigned zero. Therefore,
"ret" needs to be assigned -EINVAL at the beginning of each loop.
Fixes: e711f968c49c ("IB/srp: replace custom implementation of hex2bin()")
Fixes: 2a174df0c602 ("IB/srp: Use kstrtoull() instead of simple_strtoull()")
Fixes: 19f313438c77 ("IB/srp: Add RDMA/CM support")
Signed-off-by: Wang Yufen <wangyufen@huawei.com>
---
drivers/infiniband/ulp/srp/ib_srp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 1075c2a..1bd3559 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3352,6 +3352,7 @@ static int srp_parse_options(struct net *net, const char *buf,
sep_opt = options;
while ((p = strsep(&sep_opt, ",\n")) != NULL) {
+ ret = -EINVAL;
if (!*p)
continue;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config()
2022-11-25 12:03 [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Wang Yufen
2022-11-25 12:03 ` [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options() Wang Yufen
@ 2022-11-25 14:31 ` Andy Shevchenko
1 sibling, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2022-11-25 14:31 UTC (permalink / raw)
To: Wang Yufen
Cc: bvanassche, jgg, leon, dennis.dalessandro, linux-rdma,
linux-kernel, bart.vanassche, easwar.hariharan
On Fri, Nov 25, 2022 at 08:03:50PM +0800, Wang Yufen wrote:
> In the previous while loop, "ret" may be assigned zero. Therefore,
> "ret" needs to be assigned -EINVAL at the beginning of each loop.
...
> while (ptr < (u32 *)(dd->platform_config.data + file_length)) {
> + ret = -EINVAL;
> header1 = *ptr;
> header2 = *(ptr + 1);
> if (header1 != ~header2) {
You may do it differently and simplify even existing code, i.e.
// The following two lines to add
bali_with_einval:
ret = -EINVAL;
bail:
memset(pcfgcache, 0, sizeof(struct platform_config_cache));
return ret;
Then you convert all goto bail; to goto bail_with_einval; where it's
appropriate. And dropping some duplicative ret = -EINVAL; lines above.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options()
2022-11-25 12:03 ` [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options() Wang Yufen
@ 2022-11-25 14:37 ` Andy Shevchenko
2022-11-26 1:23 ` wangyufen
0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2022-11-25 14:37 UTC (permalink / raw)
To: Wang Yufen
Cc: bvanassche, jgg, leon, dennis.dalessandro, linux-rdma,
linux-kernel, bart.vanassche, easwar.hariharan
On Fri, Nov 25, 2022 at 08:03:51PM +0800, Wang Yufen wrote:
> In the previous while loop, "ret" may be assigned zero. Therefore,
> "ret" needs to be assigned -EINVAL at the beginning of each loop.
...
> while ((p = strsep(&sep_opt, ",\n")) != NULL) {
> + ret = -EINVAL;
> if (!*p)
> continue;
Better option is to investigate each case separately and gather all of them
into a single fix.
For example, this
case SRP_OPT_MAX_IT_IU_SIZE:
if (match_int(args, &token) || token < 0) {
pr_warn("bad maximum initiator to target IU size '%s'\n", p);
goto out;
}
target->max_it_iu_size = token;
break;
can be rewritten as
case SRP_OPT_MAX_IT_IU_SIZE:
ret = match_int(args, &token);
if (ret)
goto out;
if (token < 0) {
pr_warn("bad maximum initiator to target IU size '%s'\n", p);
ret = -EINVAL;
goto out;
}
target->max_it_iu_size = token;
break;
and so on...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options()
2022-11-25 14:37 ` Andy Shevchenko
@ 2022-11-26 1:23 ` wangyufen
0 siblings, 0 replies; 5+ messages in thread
From: wangyufen @ 2022-11-26 1:23 UTC (permalink / raw)
To: Andy Shevchenko
Cc: bvanassche, jgg, leon, dennis.dalessandro, linux-rdma,
linux-kernel, bart.vanassche, easwar.hariharan
在 2022/11/25 22:37, Andy Shevchenko 写道:
> On Fri, Nov 25, 2022 at 08:03:51PM +0800, Wang Yufen wrote:
>> In the previous while loop, "ret" may be assigned zero. Therefore,
>> "ret" needs to be assigned -EINVAL at the beginning of each loop.
>
> ...
>
>> while ((p = strsep(&sep_opt, ",\n")) != NULL) {
>> + ret = -EINVAL;
>> if (!*p)
>> continue;
>
> Better option is to investigate each case separately and gather all of them
> into a single fix.
>
> For example, this
>
> case SRP_OPT_MAX_IT_IU_SIZE:
> if (match_int(args, &token) || token < 0) {
> pr_warn("bad maximum initiator to target IU size '%s'\n", p);
> goto out;
> }
> target->max_it_iu_size = token;
> break;
>
> can be rewritten as
>
> case SRP_OPT_MAX_IT_IU_SIZE:
> ret = match_int(args, &token);
> if (ret)
> goto out;
> if (token < 0) {
> pr_warn("bad maximum initiator to target IU size '%s'\n", p);
> ret = -EINVAL;
> goto out;
> }
> target->max_it_iu_size = token;
> break;
>
> and so on...
>
I got it. Will change in v2.
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-26 1:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-25 12:03 [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Wang Yufen
2022-11-25 12:03 ` [PATCH 2/2] RDMA/srp: Fix error return code in srp_parse_options() Wang Yufen
2022-11-25 14:37 ` Andy Shevchenko
2022-11-26 1:23 ` wangyufen
2022-11-25 14:31 ` [PATCH 1/2] RDMA/hfi1: Fix error return code in parse_platform_config() Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.