On 11/20/24 09:53, Philippe Mathieu-Daudé wrote:
@@ -112,7 +113,7 @@ struct SCSIDiskState {
char *vendor;
char *product;
char *device_id;
- char *loadparm; /* only for s390x */
+ char loadparm[LOADPARM_LEN]; /* only for s390x */
You would need a +1 here because of
static char *scsi_property_get_loadparm(Object *obj, Error **errp)
{
return g_strdup(SCSI_DISK_BASE(obj)->loadparm);
}
expecting NUL-termination as well.
- lp_str = g_malloc0(strlen(value));
I have sent a pull request that simply adds the +1 here, also because...
- if (!qdev_prop_sanitize_s390x_loadparm(lp_str, value, errp)) {
- g_free(lp_str);
- return;
- }
- SCSI_DISK_BASE(obj)->loadparm = lp_str;
+ qdev_prop_sanitize_s390x_loadparm(SCSI_DISK_BASE(obj)->loadparm, value,
errp);
... this would overwrite SCSI_DISK_BASE(obj)->loadparm in case of error.
Note how the code is setting loadparm only after a successful
qdev_prop_sanitize_s390x_loadparm. That's not a problem in practice,
because failing to set a property is usually fatal, but not good style
either.
Thanks,
Paolo
