On 3/21/25 8:15 AM, Alice Guo (OSS) wrote:

[...]

@@ -276,22 +276,31 @@ static int scmi_base_discover_list_protocols_int(struct 
udevice *dev,
        if (ret)
                return ret;

This math here could really use a code comment to explain what is going on here.

I assume there is 1 byte per protocol in the struct scmi_base_discover_list_protocols_out { .protocols[] } member, right ? If so, why not simply change u32 protocols[] to u8 protocols[] and use some:

out_size = sizeof(*out) + roundup(num_protocols, sizeof(u32))

?

That might make the whole parsing using for (i = 0; i < out->num_protocols; i++, cur++) simpler too ?

+       out_size = sizeof(*out) + sizeof(u32) * (1 + (num_protocols - 1) / 4);
+       out = calloc(1, out_size);
+       if (!out)
+               return -ENOMEM;
+       msg.out_msg = (u8 *)out;
+       msg.out_msg_sz = out_size;
+
        buf = calloc(sizeof(u8), num_protocols);
-       if (!buf)
+       if (!buf) {
+               free(out);
                return -ENOMEM;
+       }
cur = 0;
        do {
                ret = devm_scmi_process_msg(dev, &msg);
                if (ret)
                        goto err;
-               if (out.status) {
-                       ret = scmi_to_linux_errno(out.status);
+               if (out->status) {
+                       ret = scmi_to_linux_errno(out->status);
                        goto err;
                }
- for (i = 0; i < out.num_protocols; i++, cur++)
-                       buf[cur] = out.protocols[i / 4] >> ((i % 4) * 8);
+               for (i = 0; i < out->num_protocols; i++, cur++)
+                       buf[cur] = out->protocols[i / 4] >> ((i % 4) * 8);
        } while (cur < num_protocols);
*protocols = buf;
@@ -299,6 +308,7 @@ static int scmi_base_discover_list_protocols_int(struct 
udevice *dev,
        return num_protocols;
  err:
        free(buf);
+       free(out);
return ret;
  }
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index 519b906b4ce..9046de7e3e7 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -145,7 +145,7 @@ struct scmi_base_discover_impl_version_out {
  struct scmi_base_discover_list_protocols_out {
        s32 status;
        u32 num_protocols;
-       u32 protocols[3];
+       u32 protocols[];
  };
[...]

Reply via email to