Re: [PATCH v5] gdb: align siginfo_t with the Linux kernel definition

[Matthieu Longo <matthieu.longo@arm.com>]

On 13/08/2026 15:48, Matthieu Longo wrote:
> On 12/08/2026 20:08, Simon Marchi wrote:
>> On 7/28/26 8:32 AM, Matthieu Longo wrote:
>>> GDB's current definition of siginfo_t is missing many fields present in
>>> the Linux kernel definition [1].
>>>
>>> These fields are useful for providing detailed, user-friendly diagnostics
>>> when a fault occurs. Some new AArch64 extensions, such as Permission
>>> Overlay Enhancement used to implement Protection Keys [2], require the
>>> debugger to inspect 'si_pkey' alongside 'si_addr' to help the user identify
>>> the problematic key.
>>>
>>> This patch aligns GDB's definition of the __sifields._sigfault member of
>>> siginfo_t with the definition from the Linux kernel master branch.
>>>
>>> To avoid hardcoding the field access paths throughout the codebase, this
>>> patch also introduces compile-time accessors for the siginfo_t attributes,
>>> centralizing their definitions in a single location and making future
>>> updates easier.
>>>
>>> Finally, extend the testsuite to verify access to the new si_pkey field
>>> and its preservation when modifying $_siginfo and when reading core files.
>>> The tests in siginfo-obj.exp rely on the siginfo_t definition provided by
>>> glibc's <signal.h>, which does not yet expose all of the fields present in
>>> the kernel definition. As a result, the tests cannot exercise every newly
>>> added field and therefore focus on si_pkey, the field motivating this change.
>>> The test validates that GDB can read and modify the field correctly; it does
>>> not attempt to generate a real protection-key fault.
>>>
>>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>>>      tree/include/uapi/asm-generic/siginfo.h#n69
>>> [2]: https://lore.kernel.org/all/20160212210213.ABC488FA@viggo.jf.intel.com/
>>>
>>> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
>>> ---
>>>  gdb/aarch64-linux-tdep.c               |  8 ++--
>>>  gdb/linux-tdep.c                       | 52 +++++++++++++++++++---
>>>  gdb/linux-tdep.h                       | 60 ++++++++++++++++++++++++++
>>>  gdb/sparc64-linux-tdep.c               |  6 ++-
>>>  gdb/testsuite/gdb.base/siginfo-obj.c   |  1 +
>>>  gdb/testsuite/gdb.base/siginfo-obj.exp | 14 ++++++
>>>  6 files changed, 131 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>>> index f11eccc1bc1..235b35bcfb4 100644
>>> --- a/gdb/aarch64-linux-tdep.c
>>> +++ b/gdb/aarch64-linux-tdep.c
>>> @@ -2683,13 +2683,15 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>>>  
>>>    try
>>>      {
>>> +      using gdb_si = gdb::siginfo_type;
>>> +      using si_key = gdb::siginfo_type::key;
>>>        /* Sigcode tells us if the segfault is actually a memory tag
>>>  	 violation.  */
>>> -      si_code = parse_and_eval_long ("$_siginfo.si_code");
>>> -      si_errno = parse_and_eval_long ("$_siginfo.si_errno");
>>> +      si_code = parse_and_eval_long (gdb_si::get (si_key::siginfo_code));
>>> +      si_errno = parse_and_eval_long (gdb_si::get (si_key::siginfo_errno));
>>>  
>>>        fault_addr
>>> -	= parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr");
>>> +	= parse_and_eval_long (gdb_si::get (si_key::siginfo_addr));
>>>      }
>>>    catch (const gdb_exception_error &exception)
>>>      {
>>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>>> index 25d625db595..740043a9292 100644
>>> --- a/gdb/linux-tdep.c
>>> +++ b/gdb/linux-tdep.c
>>> @@ -272,10 +272,9 @@ static struct type *
>>>  linux_get_siginfo_type (struct gdbarch *gdbarch)
>>>  {
>>>    struct linux_gdbarch_data *linux_gdbarch_data;
>>> -  struct type *void_ptr_type;
>>>    struct type *uid_type, *pid_type;
>>>    struct type *sigval_type, *clock_type;
>>> -  struct type *siginfo_type, *sifields_type;
>>> +  struct type *siginfo_type, *sifields_type, *sigfault_union_type;
>>>    struct type *type;
>>>  
>>>    linux_gdbarch_data = get_linux_gdbarch_data (gdbarch);
>>> @@ -285,11 +284,22 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>>    type_allocator alloc (gdbarch);
>>>  
>>>    const struct builtin_type *builtin_types = builtin_type (gdbarch);
>>> +  struct type *short_type = builtin_types->builtin_short;
>>>    struct type *int_type = builtin_types->builtin_int;
>>>    struct type *uint_type = builtin_types->builtin_unsigned_int;
>>>    struct type *long_type = builtin_types->builtin_long;
>>> -
>>> -  void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
>>> +  struct type *unsigned_long_type = builtin_types->builtin_unsigned_long;
>>> +  struct type *uint32_type = builtin_types->builtin_uint32;
>>> +  struct type *void_ptr_type
>>> +    = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
>>> +
>>> +  /* Compute padding length, i.e. __ADDR_BND_PKEY_PAD.  */
>>> +  unsigned alignof_void_ptr = type_align (void_ptr_type);
>>> +  unsigned padding_size = (alignof_void_ptr < short_type->length ()
>>> +			   ? short_type->length ()
>>> +			   : alignof_void_ptr);
>>> +  struct type *addr_bnd_pkey_padding_type
>>> +    = init_vector_type (builtin_types->builtin_uint8, padding_size);
>>>  
>>>    /* sival_t */
>>>    sigval_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
>>> @@ -364,9 +374,41 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>>    append_composite_type_field (type, "si_stime", clock_type);
>>>    append_composite_type_field (sifields_type, "_sigchld", type);
>>>  
>>> -  /* _sigfault */
>>> +  /* Begin _sigfault's anonymous union.  */
>>> +  sigfault_union_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
>>> +  /* used on alpha and sparc */
>>> +  append_composite_type_field (sigfault_union_type, "si_trapno", int_type);
>>> +  /* used when si_code is BUS_MCEERR_AR or BUS_MCEERR_AO.  */
>>> +  append_composite_type_field (sigfault_union_type, "si_addr_lsb", short_type);
>>> +
>>> +  /* used when si_code=SEGV_BNDERR */
>>> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>> +  append_composite_type_field (type, "_dummy_bnd", addr_bnd_pkey_padding_type);
>>> +  append_composite_type_field (type, "si_lower", void_ptr_type);
>>> +  append_composite_type_field (type, "si_upper", void_ptr_type);
>>> +  append_composite_type_field (sigfault_union_type, "_addr_bnd", type);
>>> +
>>> +  /* used when si_code=SEGV_PKUERR */
>>> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>> +  append_composite_type_field (type, "_dummy_pkey", addr_bnd_pkey_padding_type);
>>> +  append_composite_type_field (type, "si_pkey", uint32_type);
>>> +  append_composite_type_field (sigfault_union_type, "_addr_pkey", type);
>>> +
>>> +  /* used when si_code=TRAP_PERF */
>>> +  type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>> +  append_composite_type_field (type, "si_perf_data", unsigned_long_type);
>>> +  append_composite_type_field (type, "si_perf_type", uint32_type);
>>> +  append_composite_type_field (type, "si_perf_flags", uint32_type);
>>> +  append_composite_type_field (sigfault_union_type, "_perf", type);
>>> +
>>> +  /* End _sigfault's anonymous union.  */
>>> +
>>> +  /* _sigfault is set by SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
>>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>>    append_composite_type_field (type, "si_addr", void_ptr_type);
>>> +  /* Since there is no possibility to declare an anonymous union,
>>> +     using '_anon_union' instead.  */
>>> +  append_composite_type_field (type, "_anon_union", sigfault_union_type);
> 
> I set the name to "", and it works as you mentioned below.
> I also fixed the usages in gdb/testsuite/gdb.base/siginfo-obj.exp
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 4660772752d..bbc4009d43c 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -406,9 +406,8 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>    /* _sigfault is set by SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>    append_composite_type_field (type, "si_addr", void_ptr_type);
> -  /* Since there is no possibility to declare an anonymous union,
> -     using '_anon_union' instead.  */
> -  append_composite_type_field (type, "_anon_union", sigfault_union_type);
> +  /* Note: this is an anonymous union.  */
> +  append_composite_type_field (type, "", sigfault_union_type);
>    append_composite_type_field (sifields_type, "_sigfault", type);
> 
>    /* _sigpoll */
> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
> index 43ed38c6633..1f40756eb2f 100644
> --- a/gdb/linux-tdep.h
> +++ b/gdb/linux-tdep.h
> @@ -143,14 +143,14 @@ struct siginfo_type
> 
>        /* SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT */
>        "$_siginfo._sifields._sigfault.si_addr",
> -      "$_siginfo._sifields._sigfault._anon_union.si_trapno",
> -      "$_siginfo._sifields._sigfault._anon_union.si_addr_lsb",
> -      "$_siginfo._sifields._sigfault._anon_union._addr_bnd.si_lower",
> -      "$_siginfo._sifields._sigfault._anon_union._addr_bnd.si_upper",
> -      "$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey",
> -      "$_siginfo._sifields._sigfault._anon_union._perf.si_perf_data",
> -      "$_siginfo._sifields._sigfault._anon_union._perf.si_perf_type",
> -      "$_siginfo._sifields._sigfault._anon_union._perf.si_perf_flags",
> +      "$_siginfo._sifields._sigfault.si_trapno",
> +      "$_siginfo._sifields._sigfault.si_addr_lsb",
> +      "$_siginfo._sifields._sigfault._addr_bnd.si_lower",
> +      "$_siginfo._sifields._sigfault._addr_bnd.si_upper",
> +      "$_siginfo._sifields._sigfault._addr_pkey.si_pkey",
> +      "$_siginfo._sifields._sigfault._perf.si_perf_data",
> +      "$_siginfo._sifields._sigfault._perf.si_perf_type",
> +      "$_siginfo._sifields._sigfault._perf.si_perf_flags",
>      };
>      return paths[static_cast<size_t> (attr_)];
>    }
> diff --git a/gdb/testsuite/gdb.base/siginfo-obj.exp b/gdb/testsuite/gdb.base/siginfo-obj.exp
> index 5e36b334068..a724a634dc0 100644
> --- a/gdb/testsuite/gdb.base/siginfo-obj.exp
> +++ b/gdb/testsuite/gdb.base/siginfo-obj.exp
> @@ -115,7 +115,7 @@ gdb_test "p \$_siginfo._sifields._sigfault.si_addr = 0x666" " = \\(void \\*\\) 0
>  gdb_test "p \$_siginfo.si_errno = 666" " = 666"
>  gdb_test "p \$_siginfo.si_code = 999" " = 999"
>  gdb_test "p \$_siginfo.si_signo = 11" " = 11"
> -gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey = 123" " = 123"
> +gdb_test "p \$_siginfo._sifields._sigfault._addr_pkey.si_pkey = 123" " = 123"
> 
>  with_test_prefix "validate modified siginfo fields" {
>      gdb_test "break $bp_location"
> @@ -143,7 +143,7 @@ if {$gcore_created} {
>      gdb_test "p \$_siginfo._sifields._sigfault.si_addr" \
>         " = \\(void \\*\\) $ssi_addr" \
>         "p \$_siginfo._sifields._sigfault.si_addr from core file"
> -    gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey" \
> +    gdb_test "p \$_siginfo._sifields._sigfault._addr_pkey.si_pkey" \
>         " = $ssi_pkey" \
> -       "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey from core file"
> +       "p \$_siginfo._sifields._sigfault._addr_pkey.si_pkey from core file"
>  }
> 
> 
>>> Can you expand on why it's not possible to have an anonymous union?  It
>> is certainly possible to have anonymous unions described in DWARF, which
>> are then translated to struct types.
>>
> 
> I simply could not find how to implement it.
> It might be useful to add to the documentation of append_composite_type_field_aligned() or maybe
> better, to the comment in gdb/gdbtypes.h above the declaration of append_composite_type_field(), a
> sentence explaining what is the effect of providing an empty name.
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index dd2d24fa8e2..e4fedd886c4 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -2431,11 +2431,12 @@ extern struct type *init_pointer_type (type_allocator &alloc, int bit,
>  extern struct type *init_fixed_point_type (type_allocator &, int, int,
>                                            const char *);
> 
> -/* Helper functions to construct a struct or record type.  An
> -   initially empty type is created using arch_composite_type().
> -   Fields are then added using append_composite_type_field*().  A union
> -   type has its size set to the largest field.  A struct type has each
> -   field packed against the previous.  */
> +/* Helper functions to construct a struct or record type.  An initially empty
> +   type is created using arch_composite_type().  Fields are then added using
> +   append_composite_type_field*().
> +   A union type has its size set to the largest field.  A struct type has each
> +   field packed against the previous.
> +   If no name is specified, the type is anonymous.  */
> 
>  extern struct type *arch_composite_type (struct gdbarch *gdbarch,
>                                          const char *name, enum type_code code);
> 
>> I think that the ideal user experience would be for users to be able to
>> access fields the same way that they do in the code, that is
>> `si.si_pkey`.  All the _sigfault/_addr_pkey/etc parts are implementation
>> details that could change.
>>
>> On top of your patch, if I just delete all the internal field names, it
>> seems to work just fine, see patch below.  In the end it models
>> something like this in C:
>>
> 
> I am not against it.
> However, could this suggestion be addressed in a different patch ?
> 
> Simplifying the existing pathes to si_* values with anonymous structs would increase the impact of
> the original patch, with potentially additional testing and carefulness required for others
> architectures (for example, see gdb/nat/amd64-linux-siginfo.c L269).
> 
> Matthieu
> 
>> struct siginto_t
>> {
>>   union
>>   {
>>     // kill
>>     struct
>>     {
>>       int si_pid;
>>       int si_uid;
>>     };
>>
>>     // timer
>>     struct
>>     {
>>       int si_tid;
>>       int si_overrun;
>>       int si_sys_private;
>>     };
>>
>>     ...
>>   };
>> };
>>
>>
>> diff --git i/gdb/linux-tdep.c w/gdb/linux-tdep.c
>> index 4660772752d6..f21fa1e18578 100644
>> --- i/gdb/linux-tdep.c
>> +++ w/gdb/linux-tdep.c
>> @@ -349,21 +349,21 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "si_pid", pid_type);
>>    append_composite_type_field (type, "si_uid", uid_type);
>> -  append_composite_type_field (sifields_type, "_kill", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* _timer */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "si_tid", int_type);
>>    append_composite_type_field (type, "si_overrun", int_type);
>>    append_composite_type_field (type, "si_sigval", sigval_type);
>> -  append_composite_type_field (sifields_type, "_timer", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* _rt */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "si_pid", pid_type);
>>    append_composite_type_field (type, "si_uid", uid_type);
>>    append_composite_type_field (type, "si_sigval", sigval_type);
>> -  append_composite_type_field (sifields_type, "_rt", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* _sigchld */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>> @@ -372,7 +372,7 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>    append_composite_type_field (type, "si_status", int_type);
>>    append_composite_type_field (type, "si_utime", clock_type);
>>    append_composite_type_field (type, "si_stime", clock_type);
>> -  append_composite_type_field (sifields_type, "_sigchld", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* Begin _sigfault's anonymous union.  */
>>    sigfault_union_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
>> @@ -386,20 +386,20 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>    append_composite_type_field (type, "_dummy_bnd", addr_bnd_pkey_padding_type);
>>    append_composite_type_field (type, "si_lower", void_ptr_type);
>>    append_composite_type_field (type, "si_upper", void_ptr_type);
>> -  append_composite_type_field (sigfault_union_type, "_addr_bnd", type);
>> +  append_composite_type_field (sigfault_union_type, "", type);
>>
>>    /* used when si_code=SEGV_PKUERR */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "_dummy_pkey", addr_bnd_pkey_padding_type);
>>    append_composite_type_field (type, "si_pkey", uint32_type);
>> -  append_composite_type_field (sigfault_union_type, "_addr_pkey", type);
>> +  append_composite_type_field (sigfault_union_type, "", type);
>>
>>    /* used when si_code=TRAP_PERF */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "si_perf_data", unsigned_long_type);
>>    append_composite_type_field (type, "si_perf_type", uint32_type);
>>    append_composite_type_field (type, "si_perf_flags", uint32_type);
>> -  append_composite_type_field (sigfault_union_type, "_perf", type);
>> +  append_composite_type_field (sigfault_union_type, "", type);
>>
>>    /* End _sigfault's anonymous union.  */
>>
>> @@ -408,21 +408,21 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>    append_composite_type_field (type, "si_addr", void_ptr_type);
>>    /* Since there is no possibility to declare an anonymous union,
>>       using '_anon_union' instead.  */
>> -  append_composite_type_field (type, "_anon_union", sigfault_union_type);
>> -  append_composite_type_field (sifields_type, "_sigfault", type);
>> +  append_composite_type_field (type, "", sigfault_union_type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* _sigpoll */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "si_band", long_type);
>>    append_composite_type_field (type, "si_fd", int_type);
>> -  append_composite_type_field (sifields_type, "_sigpoll", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* _sigsys */
>>    type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>>    append_composite_type_field (type, "_call_addr", void_ptr_type);
>>    append_composite_type_field (type, "_syscall", int_type);
>>    append_composite_type_field (type, "_arch", uint_type);
>> -  append_composite_type_field (sifields_type, "_sigsys", type);
>> +  append_composite_type_field (sifields_type, "", type);
>>
>>    /* struct siginfo */
>>    siginfo_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
>> @@ -431,7 +431,7 @@ linux_get_siginfo_type (struct gdbarch *gdbarch)
>>    append_composite_type_field (siginfo_type, "si_errno", int_type);
>>    append_composite_type_field (siginfo_type, "si_code", int_type);
>>    append_composite_type_field_aligned (siginfo_type,
>> -				       "_sifields", sifields_type,
>> +				       "", sifields_type,
>>  				       long_type->length ());
>>
>>    linux_gdbarch_data->siginfo_type = siginfo_type;
>> diff --git i/gdb/testsuite/gdb.base/siginfo-obj.exp w/gdb/testsuite/gdb.base/siginfo-obj.exp
>> index 5e36b3340680..272d74ac2805 100644
>> --- i/gdb/testsuite/gdb.base/siginfo-obj.exp
>> +++ w/gdb/testsuite/gdb.base/siginfo-obj.exp
>> @@ -111,11 +111,11 @@ gdb_test "continue" ".*Program received signal SIGSEGV.*" \
>>  	 "continue to signal, 2nd"
>>
>>  set test "set si_addr"
>> -gdb_test "p \$_siginfo._sifields._sigfault.si_addr = 0x666" " = \\(void \\*\\) 0x666"
>> +gdb_test "p \$_siginfo.si_addr = 0x666" " = \\(void \\*\\) 0x666"
>>  gdb_test "p \$_siginfo.si_errno = 666" " = 666"
>>  gdb_test "p \$_siginfo.si_code = 999" " = 999"
>>  gdb_test "p \$_siginfo.si_signo = 11" " = 11"
>> -gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey = 123" " = 123"
>> +gdb_test "p \$_siginfo.si_pkey = 123" " = 123"
>>
>>  with_test_prefix "validate modified siginfo fields" {
>>      gdb_test "break $bp_location"
>> @@ -140,10 +140,10 @@ if {$gcore_created} {
>>  	"p \$_siginfo.si_errno from core file"
>>      gdb_test "p \$_siginfo.si_code" " = $ssi_code" \
>>  	"p \$_siginfo.si_code from core file"
>> -    gdb_test "p \$_siginfo._sifields._sigfault.si_addr" \
>> +    gdb_test "p \$_siginfo.si_addr" \
>>  	" = \\(void \\*\\) $ssi_addr" \
>> -	"p \$_siginfo._sifields._sigfault.si_addr from core file"
>> -    gdb_test "p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey" \
>> +	"p \$_siginfo.si_addr from core file"
>> +    gdb_test "p \$_siginfo.si_pkey" \
>>  	" = $ssi_pkey" \
>> -	"p \$_siginfo._sifields._sigfault._anon_union._addr_pkey.si_pkey from core file"
>> +	"p \$_siginfo.si_pkey from core file"
>>  }
>>
>> Simon
> 

Hi Simon,

Do you want me to publish a new revision of the patch ?
The things you raised looked minor to me, so I would assume that I don't need to publish a new revision.
Please let me know if you think otherwise.

Regards,
Matthieu