Hi Jes,
On 08.10.2021 18:05, Jes Sorensen wrote:
On 9/20/21 7:10 AM, Mateusz Grzonka wrote:
Returning NULL from get_imsm_dev or __get_imsm_dev will cause segfault.
Guarantee that it never happens.
Signed-off-by: Mateusz Grzonka <mateusz.grzonka@xxxxxxxxx>
---
super-intel.c | 122 +++++++++++++++++++-------------------------------
1 file changed, 46 insertions(+), 76 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 83ddc000..2c3df58a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -858,30 +858,29 @@ static struct imsm_dev *__get_imsm_dev(struct imsm_super *mpb, __u8 index)
void *_mpb = mpb;
if (index >= mpb->num_raid_devs)
- return NULL;
+ goto error;
/* devices start after all disks */
offset = ((void *) &mpb->disk[mpb->num_disks]) - _mpb;
- for (i = 0; i <= index; i++)
+ for (i = 0; i <= index; i++, offset += sizeof_imsm_dev(_mpb + offset, 0))
if (i == index)
return _mpb + offset;
- else
- offset += sizeof_imsm_dev(_mpb + offset, 0);
-
- return NULL;
+error:
+ pr_err("matching failed, index: %u\n", index);
+ abort();
}
Can we please fix the error handling properly instead of throwing in
abort() and assert() to avoid it. That's really sloppy in my book.
This situation is unexpected and shall never happen. I don't see any
advantage of handling it. Abort() is the most elegant solution.
If it happens, it shall fail loudly. Verifying that we don't get
NULL in each place where it is called is harmful and unnecessary.
Passing wrong index means that code is broken and we definitely cannot
continue.
Thanks,
Mariusz
