On 16.1.2013 13:53, Lucas De Marchi wrote:
> On Wed, Jan 16, 2013 at 7:18 AM, Michal Marek <mmarek@xxxxxxx> wrote:
> You forgot to update the docs. The doc says it gets the info from
> .modinfo section. This is not true for the signature, which is in the
> end of the module.
Ah, right, I will update it.
>> + if (kmod_module_signature_info(mod->file, &sig_info)) {
>> + struct kmod_list *n;
>> + char *key_hex;
>> +
>> + n = kmod_module_info_append(list, "signer", strlen("signer"),
>> + sig_info.signer, sig_info.signer_len);
>> + if (n == NULL)
>> + goto list_error;
>
> and here. Also I didn't notice in the first patch that you are freeing
> and setting *list to NULL on ENOMEM. Please add this after label
> list_error.
Right, doing this at the end of kmod_module_get_info() would more elegant.
>> + }
>> + for (i = 0; i < (int)sig_info.key_id_len; i++) {
>> + sprintf(key_hex + i * 3, "%02X",
>> + (unsigned char)sig_info.key_id[i]);
>
> no need for this cast?
The argument is promoted to int. On architectures with signed char,
values >= 0x80 would end up as 0xFFnn. The other option is to change
key_id to unsigned char pointer, but this would require a cast in
kmod_module_signature_info():
sig_info->key_id = (unsigned char *)(mem + size);
Let me know what you prefer.
>> +int kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info)
>
> please make this bool since we only return true or false.
Will do.
On 16.1.2013 13:57, Lucas De Marchi wrote:
> ohh... I and forgot to say. Please add a test under testsuite:
> testsuite/test-modinfo.c adding the module and correct output to
> rootfs-pristine.
I am going to do this. I just did not want to spam the list with a large
patch adding new *.ko files in the first iteration.
Thanks a lot for the review,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
