Thanks for the feedback!
On 5/22/26 4:31 PM, Collin Walling wrote:
> On 5/5/26 16:18, Zhuoying Cai wrote:
>> Create a certificate store for boot certificates used for secure IPL.
>>
>> Load certificates from the `boot-certs` parameter of s390-ccw-virtio
>> machine type option into the cert store.
>>
>> Currently, only X.509 certificates in PEM format are supported, as the
>> QEMU command line accepts certificates in PEM format only.
>
> Let's add more context here:
>
> "The raw Base64 data is stored, as well as the certificate's size. The
> binary (DER) size is stored as well, which may later be utilized for
> secure boot (signature verification)."
>
>>
>> Signed-off-by: Zhuoying Cai <[email protected]>
>> Reviewed-by: Farhan Ali<[email protected]>
[...]
>> +
>> +static void update_cert_store(S390IPLCertificateStore *cert_store,
>> + S390IPLCertificate *cert)
>> +{
>> + size_t data_buf_size;
>> + size_t keyid_buf_size;
>> + size_t hash_buf_size;
>> + size_t cert_buf_size;
>> +
>> + /* length field is word aligned for later DIAG use */
>> + keyid_buf_size = ROUND_UP(CERT_KEY_ID_LEN, 4);
>> + hash_buf_size = ROUND_UP(CERT_HASH_LEN, 4);
>> + cert_buf_size = ROUND_UP(cert->der_size, 4);
>> + data_buf_size = keyid_buf_size + hash_buf_size + cert_buf_size;
>> +
>> + if (cert_store->largest_cert_size < data_buf_size) {
>> + cert_store->largest_cert_size = data_buf_size;
>> + }
>> +
>> + g_assert(cert_store->count < MAX_CERTIFICATES);
>> +
>> + cert_store->certs[cert_store->count] = *cert;
>
> Should a g_steal_pointer be used here?
>
No, g_steal_pointer is not needed here. This line performs a struct
copy, not a pointer assignment and no ownership transfer. The entire
S390IPLCertificate is copied by value into this array.
>> + cert_store->total_bytes += data_buf_size;
>> + cert_store->count++;
>> +}
>> +
>
> Functionally, things look good to me.
>
