Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 )
Change subject: layout: Check return values for strdup in register_include_arg ......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70006/comment/bf7f8ba8_4310e870 PS2, Line 11: Explain the issues and the new code changes.
File layout.c:
https://review.coreboot.org/c/flashrom/+/70006/comment/23b45f39_49c20fb8 PS2, Line 139: || file == NULL This is wrong. `file` is allowed to be NULL since it's optional (see line 130-135). That's why it's set to NULL in line 137 when no colon was found.
Line 137 needs to be written differently so that we can differentiate between NULL returned by strdup and NULL that is allowed. We need an extra if for that and check for allocated memory separately.
1. Set `file` in line 123 to NULL as default value.
2. Move this if-statement above line 137 and remove `file` from the condition. ``` name = colon ? strndup(arg, colon - arg) : strdup(arg); if (!name) { msg_gerr("Could not allocate memory.\n"); goto error; } ```
3. Rewrite line 137, and I also would merge it with lines 132-135 so that we don't check for `colon` twice. ``` if (colon) { if (!colon[1]) { msg_gerr("Missing filename parameter in %s\n", arg); return 1; }
file = strdup(colon + 1); if (!file) { msg_gerr("Could not allocate memory.\n"); goto error; } } ```