Re: [Xen-devel] [livepatch-build-tools part2 3/6] create-diff-object: Add is_special_section() helper function

["Wieczorkiewicz, Pawel" <wipawel@xxxxxxxxx>]

> On 29. Apr 2019, at 17:25, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> wrote:
> 
> On 4/16/19 1:07 PM, Pawel Wieczorkiewicz wrote:
>> This function determines, based on the given section name, if the
>> sections belongs to the special sections category.
>> It treats a name from the special_sections array as a prefix. Any
>> sections whose name starts with special section prefix is considered
>> a special section.
>> Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
>> Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
>> Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>
>> Reviewed-by: Norbert Manthey <nmanthey@xxxxxxxxx>
>> ---
>>  common.c | 23 +++++++++++++++++++++++
>>  common.h |  1 +
>>  2 files changed, 24 insertions(+)
>> diff --git a/common.c b/common.c
>> index c968299..a2c860b 100644
>> --- a/common.c
>> +++ b/common.c
>> @@ -290,6 +290,29 @@ int is_referenced_section(const struct section *sec, 
>> const struct kpatch_elf *ke
>>      return false;
>>  }
>>  +int is_special_section(struct section *sec)
> 
> const for the parameter?

ACK. Will add.

> 
>> +{
>> +    static struct special_section_names {
>> +            const char *name;
>> +    } names[] = {
>> +            { .name         = ".bug_frames"                 },
>> +            { .name         = ".fixup"                      },
>> +            { .name         = ".ex_table"                   },
>> +            { .name         = ".altinstructions"            },
>> +            { .name         = ".altinstr_replacement"       },
>> +            { .name         = ".livepatch.hooks"            },
>> +            { .name         = NULL                          },
>> +    };
>> +    struct special_section_names *special;
> 
> Wouldn't it be better to reuse the existing special_sections array rather 
> than partially duplicating it here?

After looking at this code again, I think you’re right and it would be better.
It would require to explicitly call out all sections to be considered special in
the special_sections array, but this is actually what I need for further 
changes anyway.

ACK. Will fix. 

> 
>> +
>> +    for (special = names; special->name; special++) {
>> +            if (!strncmp(sec->name, special->name, strlen(special->name)))
>> +                    return true;
> 
> This check is not as precise as it could be, since ".altinstructionsfoo" 
> would be considered special when it actually means nothing to this tool.

Given the above reasoning, that’s right.

ACK. Will fix.

> 
> -- 
> Ross Lagerwall


Best Regards,
Pawel Wieczorkiewicz



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel