public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Rename the read symbol to xread
@ 2020-03-17 12:45 Kamil Rytarowski
  2020-03-17 17:51 ` Christian Biesinger
  0 siblings, 1 reply; 10+ messages in thread
From: Kamil Rytarowski @ 2020-03-17 12:45 UTC (permalink / raw)
  To: gdb-patches

This avoids clashes with macro read in the NetBSD headers.
---
 gdb/user-regs.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/user-regs.c b/gdb/user-regs.c
index a232b1eb000..a968a18bc23 100644
--- a/gdb/user-regs.c
+++ b/gdb/user-regs.c
@@ -41,7 +41,7 @@
 struct user_reg
 {
   const char *name;
-  struct value *(*read) (struct frame_info * frame, const void *baton);
+  struct value *(*xread) (struct frame_info * frame, const void *baton);
   const void *baton;
   struct user_reg *next;
 };
@@ -60,7 +60,7 @@ struct gdb_user_regs

 static void
 append_user_reg (struct gdb_user_regs *regs, const char *name,
-		 user_reg_read_ftype *read, const void *baton,
+		 user_reg_read_ftype *xread, const void *baton,
 		 struct user_reg *reg)
 {
   /* The caller is responsible for allocating memory needed to store
@@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
      register list stored in the common heap or a specific obstack.  */
   gdb_assert (reg != NULL);
   reg->name = name;
-  reg->read = read;
+  reg->xread = xread;
   reg->baton = baton;
   reg->next = NULL;
   (*regs->last) = reg;
@@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
 };

 void
-user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
+user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
 		      const void *baton)
 {
-  append_user_reg (&builtin_user_regs, name, read, baton,
+  append_user_reg (&builtin_user_regs, name, xread, baton,
 		   XNEW (struct user_reg));
 }

@@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)

   regs->last = &regs->first;
   for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
-    append_user_reg (regs, reg->name, reg->read, reg->baton,
+    append_user_reg (regs, reg->name, reg->xread, reg->baton,
 		     GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
   return regs;
 }

 void
 user_reg_add (struct gdbarch *gdbarch, const char *name,
-	      user_reg_read_ftype *read, const void *baton)
+	      user_reg_read_ftype *xread, const void *baton)
 {
   struct gdb_user_regs *regs
     = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
@@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
       regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
       deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
     }
-  append_user_reg (regs, name, read, baton,
+  append_user_reg (regs, name, xread, baton,
 		   GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
 }

@@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
   struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);

   gdb_assert (reg != NULL);
-  return reg->read (frame, reg->baton);
+  return reg->xread (frame, reg->baton);
 }

 static void
--
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Rename the read symbol to xread
  2020-03-17 12:45 [PATCH] Rename the read symbol to xread Kamil Rytarowski
@ 2020-03-17 17:51 ` Christian Biesinger
  2020-03-17 17:58   ` Kamil Rytarowski
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Biesinger @ 2020-03-17 17:51 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

On Tue, Mar 17, 2020 at 7:46 AM Kamil Rytarowski <n54@gmx.com> wrote:
>
> This avoids clashes with macro read in the NetBSD headers.

Out of curiosity, what does NetBSD define read to?

I think it would be useful to add a comment saying why this is called xread.

Christian

> ---
>  gdb/user-regs.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/user-regs.c b/gdb/user-regs.c
> index a232b1eb000..a968a18bc23 100644
> --- a/gdb/user-regs.c
> +++ b/gdb/user-regs.c
> @@ -41,7 +41,7 @@
>  struct user_reg
>  {
>    const char *name;
> -  struct value *(*read) (struct frame_info * frame, const void *baton);
> +  struct value *(*xread) (struct frame_info * frame, const void *baton);
>    const void *baton;
>    struct user_reg *next;
>  };
> @@ -60,7 +60,7 @@ struct gdb_user_regs
>
>  static void
>  append_user_reg (struct gdb_user_regs *regs, const char *name,
> -                user_reg_read_ftype *read, const void *baton,
> +                user_reg_read_ftype *xread, const void *baton,
>                  struct user_reg *reg)
>  {
>    /* The caller is responsible for allocating memory needed to store
> @@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
>       register list stored in the common heap or a specific obstack.  */
>    gdb_assert (reg != NULL);
>    reg->name = name;
> -  reg->read = read;
> +  reg->xread = xread;
>    reg->baton = baton;
>    reg->next = NULL;
>    (*regs->last) = reg;
> @@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
>  };
>
>  void
> -user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
> +user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
>                       const void *baton)
>  {
> -  append_user_reg (&builtin_user_regs, name, read, baton,
> +  append_user_reg (&builtin_user_regs, name, xread, baton,
>                    XNEW (struct user_reg));
>  }
>
> @@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)
>
>    regs->last = &regs->first;
>    for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
> -    append_user_reg (regs, reg->name, reg->read, reg->baton,
> +    append_user_reg (regs, reg->name, reg->xread, reg->baton,
>                      GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>    return regs;
>  }
>
>  void
>  user_reg_add (struct gdbarch *gdbarch, const char *name,
> -             user_reg_read_ftype *read, const void *baton)
> +             user_reg_read_ftype *xread, const void *baton)
>  {
>    struct gdb_user_regs *regs
>      = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
> @@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
>        regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
>        deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
>      }
> -  append_user_reg (regs, name, read, baton,
> +  append_user_reg (regs, name, xread, baton,
>                    GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>  }
>
> @@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
>    struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);
>
>    gdb_assert (reg != NULL);
> -  return reg->read (frame, reg->baton);
> +  return reg->xread (frame, reg->baton);
>  }
>
>  static void
> --
> 2.25.0
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Rename the read symbol to xread
  2020-03-17 17:51 ` Christian Biesinger
@ 2020-03-17 17:58   ` Kamil Rytarowski
  2020-03-17 18:00     ` Christian Biesinger
  0 siblings, 1 reply; 10+ messages in thread
From: Kamil Rytarowski @ 2020-03-17 17:58 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 3836 bytes --]

On 17.03.2020 18:51, Christian Biesinger wrote:
> On Tue, Mar 17, 2020 at 7:46 AM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> This avoids clashes with macro read in the NetBSD headers.
> 
> Out of curiosity, what does NetBSD define read to?
> 
> I think it would be useful to add a comment saying why this is called xread.
> 

NetBSD ships with SSP (stack smashing protector) support that
conditionally redefines read(2).

https://github.com/NetBSD/src/blob/trunk/include/ssp/unistd.h#L39

Is a comment in the commit message enough?

> Christian
> 
>> ---
>>  gdb/user-regs.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/gdb/user-regs.c b/gdb/user-regs.c
>> index a232b1eb000..a968a18bc23 100644
>> --- a/gdb/user-regs.c
>> +++ b/gdb/user-regs.c
>> @@ -41,7 +41,7 @@
>>  struct user_reg
>>  {
>>    const char *name;
>> -  struct value *(*read) (struct frame_info * frame, const void *baton);
>> +  struct value *(*xread) (struct frame_info * frame, const void *baton);
>>    const void *baton;
>>    struct user_reg *next;
>>  };
>> @@ -60,7 +60,7 @@ struct gdb_user_regs
>>
>>  static void
>>  append_user_reg (struct gdb_user_regs *regs, const char *name,
>> -                user_reg_read_ftype *read, const void *baton,
>> +                user_reg_read_ftype *xread, const void *baton,
>>                  struct user_reg *reg)
>>  {
>>    /* The caller is responsible for allocating memory needed to store
>> @@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
>>       register list stored in the common heap or a specific obstack.  */
>>    gdb_assert (reg != NULL);
>>    reg->name = name;
>> -  reg->read = read;
>> +  reg->xread = xread;
>>    reg->baton = baton;
>>    reg->next = NULL;
>>    (*regs->last) = reg;
>> @@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
>>  };
>>
>>  void
>> -user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
>> +user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
>>                       const void *baton)
>>  {
>> -  append_user_reg (&builtin_user_regs, name, read, baton,
>> +  append_user_reg (&builtin_user_regs, name, xread, baton,
>>                    XNEW (struct user_reg));
>>  }
>>
>> @@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)
>>
>>    regs->last = &regs->first;
>>    for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
>> -    append_user_reg (regs, reg->name, reg->read, reg->baton,
>> +    append_user_reg (regs, reg->name, reg->xread, reg->baton,
>>                      GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>>    return regs;
>>  }
>>
>>  void
>>  user_reg_add (struct gdbarch *gdbarch, const char *name,
>> -             user_reg_read_ftype *read, const void *baton)
>> +             user_reg_read_ftype *xread, const void *baton)
>>  {
>>    struct gdb_user_regs *regs
>>      = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
>> @@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
>>        regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
>>        deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
>>      }
>> -  append_user_reg (regs, name, read, baton,
>> +  append_user_reg (regs, name, xread, baton,
>>                    GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>>  }
>>
>> @@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
>>    struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);
>>
>>    gdb_assert (reg != NULL);
>> -  return reg->read (frame, reg->baton);
>> +  return reg->xread (frame, reg->baton);
>>  }
>>
>>  static void
>> --
>> 2.25.0
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Rename the read symbol to xread
  2020-03-17 17:58   ` Kamil Rytarowski
@ 2020-03-17 18:00     ` Christian Biesinger
  2020-03-17 18:01       ` Kamil Rytarowski
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Biesinger @ 2020-03-17 18:00 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

On Tue, Mar 17, 2020 at 12:59 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 17.03.2020 18:51, Christian Biesinger wrote:
> > On Tue, Mar 17, 2020 at 7:46 AM Kamil Rytarowski <n54@gmx.com> wrote:
> >>
> >> This avoids clashes with macro read in the NetBSD headers.
> >
> > Out of curiosity, what does NetBSD define read to?
> >
> > I think it would be useful to add a comment saying why this is called xread.
> >
>
> NetBSD ships with SSP (stack smashing protector) support that
> conditionally redefines read(2).
>
> https://github.com/NetBSD/src/blob/trunk/include/ssp/unistd.h#L39

Ah thanks.

> Is a comment in the commit message enough?

Not IMO, that's much harder to find...

Christian

> > Christian
> >
> >> ---
> >>  gdb/user-regs.c | 18 +++++++++---------
> >>  1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/gdb/user-regs.c b/gdb/user-regs.c
> >> index a232b1eb000..a968a18bc23 100644
> >> --- a/gdb/user-regs.c
> >> +++ b/gdb/user-regs.c
> >> @@ -41,7 +41,7 @@
> >>  struct user_reg
> >>  {
> >>    const char *name;
> >> -  struct value *(*read) (struct frame_info * frame, const void *baton);
> >> +  struct value *(*xread) (struct frame_info * frame, const void *baton);
> >>    const void *baton;
> >>    struct user_reg *next;
> >>  };
> >> @@ -60,7 +60,7 @@ struct gdb_user_regs
> >>
> >>  static void
> >>  append_user_reg (struct gdb_user_regs *regs, const char *name,
> >> -                user_reg_read_ftype *read, const void *baton,
> >> +                user_reg_read_ftype *xread, const void *baton,
> >>                  struct user_reg *reg)
> >>  {
> >>    /* The caller is responsible for allocating memory needed to store
> >> @@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
> >>       register list stored in the common heap or a specific obstack.  */
> >>    gdb_assert (reg != NULL);
> >>    reg->name = name;
> >> -  reg->read = read;
> >> +  reg->xread = xread;
> >>    reg->baton = baton;
> >>    reg->next = NULL;
> >>    (*regs->last) = reg;
> >> @@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
> >>  };
> >>
> >>  void
> >> -user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
> >> +user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
> >>                       const void *baton)
> >>  {
> >> -  append_user_reg (&builtin_user_regs, name, read, baton,
> >> +  append_user_reg (&builtin_user_regs, name, xread, baton,
> >>                    XNEW (struct user_reg));
> >>  }
> >>
> >> @@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)
> >>
> >>    regs->last = &regs->first;
> >>    for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
> >> -    append_user_reg (regs, reg->name, reg->read, reg->baton,
> >> +    append_user_reg (regs, reg->name, reg->xread, reg->baton,
> >>                      GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
> >>    return regs;
> >>  }
> >>
> >>  void
> >>  user_reg_add (struct gdbarch *gdbarch, const char *name,
> >> -             user_reg_read_ftype *read, const void *baton)
> >> +             user_reg_read_ftype *xread, const void *baton)
> >>  {
> >>    struct gdb_user_regs *regs
> >>      = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
> >> @@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
> >>        regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
> >>        deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
> >>      }
> >> -  append_user_reg (regs, name, read, baton,
> >> +  append_user_reg (regs, name, xread, baton,
> >>                    GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
> >>  }
> >>
> >> @@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
> >>    struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);
> >>
> >>    gdb_assert (reg != NULL);
> >> -  return reg->read (frame, reg->baton);
> >> +  return reg->xread (frame, reg->baton);
> >>  }
> >>
> >>  static void
> >> --
> >> 2.25.0
> >>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Rename the read symbol to xread
  2020-03-17 18:00     ` Christian Biesinger
@ 2020-03-17 18:01       ` Kamil Rytarowski
  2020-03-17 18:59         ` Christian Biesinger
  0 siblings, 1 reply; 10+ messages in thread
From: Kamil Rytarowski @ 2020-03-17 18:01 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 4313 bytes --]

On 17.03.2020 19:00, Christian Biesinger wrote:
> On Tue, Mar 17, 2020 at 12:59 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 17.03.2020 18:51, Christian Biesinger wrote:
>>> On Tue, Mar 17, 2020 at 7:46 AM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>
>>>> This avoids clashes with macro read in the NetBSD headers.
>>>
>>> Out of curiosity, what does NetBSD define read to?
>>>
>>> I think it would be useful to add a comment saying why this is called xread.
>>>
>>
>> NetBSD ships with SSP (stack smashing protector) support that
>> conditionally redefines read(2).
>>
>> https://github.com/NetBSD/src/blob/trunk/include/ssp/unistd.h#L39
> 
> Ah thanks.
> 
>> Is a comment in the commit message enough?
> 
> Not IMO, that's much harder to find...
> 

Where should I document this? in struct_reg?

> Christian
> 
>>> Christian
>>>
>>>> ---
>>>>  gdb/user-regs.c | 18 +++++++++---------
>>>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/gdb/user-regs.c b/gdb/user-regs.c
>>>> index a232b1eb000..a968a18bc23 100644
>>>> --- a/gdb/user-regs.c
>>>> +++ b/gdb/user-regs.c
>>>> @@ -41,7 +41,7 @@
>>>>  struct user_reg
>>>>  {
>>>>    const char *name;
>>>> -  struct value *(*read) (struct frame_info * frame, const void *baton);
>>>> +  struct value *(*xread) (struct frame_info * frame, const void *baton);
>>>>    const void *baton;
>>>>    struct user_reg *next;
>>>>  };
>>>> @@ -60,7 +60,7 @@ struct gdb_user_regs
>>>>
>>>>  static void
>>>>  append_user_reg (struct gdb_user_regs *regs, const char *name,
>>>> -                user_reg_read_ftype *read, const void *baton,
>>>> +                user_reg_read_ftype *xread, const void *baton,
>>>>                  struct user_reg *reg)
>>>>  {
>>>>    /* The caller is responsible for allocating memory needed to store
>>>> @@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
>>>>       register list stored in the common heap or a specific obstack.  */
>>>>    gdb_assert (reg != NULL);
>>>>    reg->name = name;
>>>> -  reg->read = read;
>>>> +  reg->xread = xread;
>>>>    reg->baton = baton;
>>>>    reg->next = NULL;
>>>>    (*regs->last) = reg;
>>>> @@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
>>>>  };
>>>>
>>>>  void
>>>> -user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
>>>> +user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
>>>>                       const void *baton)
>>>>  {
>>>> -  append_user_reg (&builtin_user_regs, name, read, baton,
>>>> +  append_user_reg (&builtin_user_regs, name, xread, baton,
>>>>                    XNEW (struct user_reg));
>>>>  }
>>>>
>>>> @@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)
>>>>
>>>>    regs->last = &regs->first;
>>>>    for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
>>>> -    append_user_reg (regs, reg->name, reg->read, reg->baton,
>>>> +    append_user_reg (regs, reg->name, reg->xread, reg->baton,
>>>>                      GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>>>>    return regs;
>>>>  }
>>>>
>>>>  void
>>>>  user_reg_add (struct gdbarch *gdbarch, const char *name,
>>>> -             user_reg_read_ftype *read, const void *baton)
>>>> +             user_reg_read_ftype *xread, const void *baton)
>>>>  {
>>>>    struct gdb_user_regs *regs
>>>>      = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
>>>> @@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
>>>>        regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
>>>>        deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
>>>>      }
>>>> -  append_user_reg (regs, name, read, baton,
>>>> +  append_user_reg (regs, name, xread, baton,
>>>>                    GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
>>>>  }
>>>>
>>>> @@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
>>>>    struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);
>>>>
>>>>    gdb_assert (reg != NULL);
>>>> -  return reg->read (frame, reg->baton);
>>>> +  return reg->xread (frame, reg->baton);
>>>>  }
>>>>
>>>>  static void
>>>> --
>>>> 2.25.0
>>>>
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Rename the read symbol to xread
  2020-03-17 18:01       ` Kamil Rytarowski
@ 2020-03-17 18:59         ` Christian Biesinger
  2020-03-17 23:57           ` [PATCH v2] " Kamil Rytarowski
  0 siblings, 1 reply; 10+ messages in thread
From: Christian Biesinger @ 2020-03-17 18:59 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

On Tue, Mar 17, 2020 at 1:02 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 17.03.2020 19:00, Christian Biesinger wrote:
> > On Tue, Mar 17, 2020 at 12:59 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>
> >> On 17.03.2020 18:51, Christian Biesinger wrote:
> >>> On Tue, Mar 17, 2020 at 7:46 AM Kamil Rytarowski <n54@gmx.com> wrote:
> >>>>
> >>>> This avoids clashes with macro read in the NetBSD headers.
> >>>
> >>> Out of curiosity, what does NetBSD define read to?
> >>>
> >>> I think it would be useful to add a comment saying why this is called xread.
> >>>
> >>
> >> NetBSD ships with SSP (stack smashing protector) support that
> >> conditionally redefines read(2).
> >>
> >> https://github.com/NetBSD/src/blob/trunk/include/ssp/unistd.h#L39
> >
> > Ah thanks.
> >
> >> Is a comment in the commit message enough?
> >
> > Not IMO, that's much harder to find...
> >
>
> Where should I document this? in struct_reg?

I'd put it above the declaration in user_reg, ...

>
> > Christian
> >
> >>> Christian
> >>>
> >>>> ---
> >>>>  gdb/user-regs.c | 18 +++++++++---------
> >>>>  1 file changed, 9 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/gdb/user-regs.c b/gdb/user-regs.c
> >>>> index a232b1eb000..a968a18bc23 100644
> >>>> --- a/gdb/user-regs.c
> >>>> +++ b/gdb/user-regs.c
> >>>> @@ -41,7 +41,7 @@
> >>>>  struct user_reg
> >>>>  {
> >>>>    const char *name;
> >>>> -  struct value *(*read) (struct frame_info * frame, const void *baton);
> >>>> +  struct value *(*xread) (struct frame_info * frame, const void *baton);

... above this line.

> >>>>    const void *baton;
> >>>>    struct user_reg *next;
> >>>>  };
> >>>> @@ -60,7 +60,7 @@ struct gdb_user_regs
> >>>>
> >>>>  static void
> >>>>  append_user_reg (struct gdb_user_regs *regs, const char *name,
> >>>> -                user_reg_read_ftype *read, const void *baton,
> >>>> +                user_reg_read_ftype *xread, const void *baton,
> >>>>                  struct user_reg *reg)
> >>>>  {
> >>>>    /* The caller is responsible for allocating memory needed to store
> >>>> @@ -68,7 +68,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
> >>>>       register list stored in the common heap or a specific obstack.  */
> >>>>    gdb_assert (reg != NULL);
> >>>>    reg->name = name;
> >>>> -  reg->read = read;
> >>>> +  reg->xread = xread;
> >>>>    reg->baton = baton;
> >>>>    reg->next = NULL;
> >>>>    (*regs->last) = reg;
> >>>> @@ -82,10 +82,10 @@ static struct gdb_user_regs builtin_user_regs = {
> >>>>  };
> >>>>
> >>>>  void
> >>>> -user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
> >>>> +user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
> >>>>                       const void *baton)
> >>>>  {
> >>>> -  append_user_reg (&builtin_user_regs, name, read, baton,
> >>>> +  append_user_reg (&builtin_user_regs, name, xread, baton,
> >>>>                    XNEW (struct user_reg));
> >>>>  }
> >>>>
> >>>> @@ -103,14 +103,14 @@ user_regs_init (struct gdbarch *gdbarch)
> >>>>
> >>>>    regs->last = &regs->first;
> >>>>    for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
> >>>> -    append_user_reg (regs, reg->name, reg->read, reg->baton,
> >>>> +    append_user_reg (regs, reg->name, reg->xread, reg->baton,
> >>>>                      GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
> >>>>    return regs;
> >>>>  }
> >>>>
> >>>>  void
> >>>>  user_reg_add (struct gdbarch *gdbarch, const char *name,
> >>>> -             user_reg_read_ftype *read, const void *baton)
> >>>> +             user_reg_read_ftype *xread, const void *baton)
> >>>>  {
> >>>>    struct gdb_user_regs *regs
> >>>>      = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
> >>>> @@ -122,7 +122,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
> >>>>        regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
> >>>>        deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
> >>>>      }
> >>>> -  append_user_reg (regs, name, read, baton,
> >>>> +  append_user_reg (regs, name, xread, baton,
> >>>>                    GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
> >>>>  }
> >>>>
> >>>> @@ -214,7 +214,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
> >>>>    struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);
> >>>>
> >>>>    gdb_assert (reg != NULL);
> >>>> -  return reg->read (frame, reg->baton);
> >>>> +  return reg->xread (frame, reg->baton);
> >>>>  }
> >>>>
> >>>>  static void
> >>>> --
> >>>> 2.25.0
> >>>>
> >>
> >>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] Rename the read symbol to xread
  2020-03-17 18:59         ` Christian Biesinger
@ 2020-03-17 23:57           ` Kamil Rytarowski
  2020-03-18  0:43             ` Simon Marchi
  0 siblings, 1 reply; 10+ messages in thread
From: Kamil Rytarowski @ 2020-03-17 23:57 UTC (permalink / raw)
  To: gdb-patches

This avoids clashes with macro read in the NetBSD headers.
---
 gdb/user-regs.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/gdb/user-regs.c b/gdb/user-regs.c
index a232b1eb000..cb922313b0c 100644
--- a/gdb/user-regs.c
+++ b/gdb/user-regs.c
@@ -41,7 +41,10 @@
 struct user_reg
 {
   const char *name;
-  struct value *(*read) (struct frame_info * frame, const void *baton);
+  /* Avoid the "read" symbol name as it conflicts with a preprocessor symbol
+     in the NetBSD header for Stack Smashing Protection, that wraps the read(2)
+     syscall.  */
+  struct value *(*xread) (struct frame_info * frame, const void *baton);
   const void *baton;
   struct user_reg *next;
 };
@@ -60,7 +63,7 @@ struct gdb_user_regs

 static void
 append_user_reg (struct gdb_user_regs *regs, const char *name,
-		 user_reg_read_ftype *read, const void *baton,
+		 user_reg_read_ftype *xread, const void *baton,
 		 struct user_reg *reg)
 {
   /* The caller is responsible for allocating memory needed to store
@@ -68,7 +71,7 @@ append_user_reg (struct gdb_user_regs *regs, const char *name,
      register list stored in the common heap or a specific obstack.  */
   gdb_assert (reg != NULL);
   reg->name = name;
-  reg->read = read;
+  reg->xread = xread;
   reg->baton = baton;
   reg->next = NULL;
   (*regs->last) = reg;
@@ -82,10 +85,10 @@ static struct gdb_user_regs builtin_user_regs = {
 };

 void
-user_reg_add_builtin (const char *name, user_reg_read_ftype *read,
+user_reg_add_builtin (const char *name, user_reg_read_ftype *xread,
 		      const void *baton)
 {
-  append_user_reg (&builtin_user_regs, name, read, baton,
+  append_user_reg (&builtin_user_regs, name, xread, baton,
 		   XNEW (struct user_reg));
 }

@@ -103,14 +106,14 @@ user_regs_init (struct gdbarch *gdbarch)

   regs->last = &regs->first;
   for (reg = builtin_user_regs.first; reg != NULL; reg = reg->next)
-    append_user_reg (regs, reg->name, reg->read, reg->baton,
+    append_user_reg (regs, reg->name, reg->xread, reg->baton,
 		     GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
   return regs;
 }

 void
 user_reg_add (struct gdbarch *gdbarch, const char *name,
-	      user_reg_read_ftype *read, const void *baton)
+	      user_reg_read_ftype *xread, const void *baton)
 {
   struct gdb_user_regs *regs
     = (struct gdb_user_regs *) gdbarch_data (gdbarch, user_regs_data);
@@ -122,7 +125,7 @@ user_reg_add (struct gdbarch *gdbarch, const char *name,
       regs = (struct gdb_user_regs *) user_regs_init (gdbarch);
       deprecated_set_gdbarch_data (gdbarch, user_regs_data, regs);
     }
-  append_user_reg (regs, name, read, baton,
+  append_user_reg (regs, name, xread, baton,
 		   GDBARCH_OBSTACK_ZALLOC (gdbarch, struct user_reg));
 }

@@ -214,7 +217,7 @@ value_of_user_reg (int regnum, struct frame_info *frame)
   struct user_reg *reg = usernum_to_user_reg (gdbarch, regnum - maxregs);

   gdb_assert (reg != NULL);
-  return reg->read (frame, reg->baton);
+  return reg->xread (frame, reg->baton);
 }

 static void
--
2.25.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Rename the read symbol to xread
  2020-03-17 23:57           ` [PATCH v2] " Kamil Rytarowski
@ 2020-03-18  0:43             ` Simon Marchi
  2020-03-18  1:01               ` Kamil Rytarowski
  2020-03-18 20:51               ` Tom Tromey
  0 siblings, 2 replies; 10+ messages in thread
From: Simon Marchi @ 2020-03-18  0:43 UTC (permalink / raw)
  To: Kamil Rytarowski, gdb-patches

On 2020-03-17 7:57 p.m., Kamil Rytarowski wrote:
> This avoids clashes with macro read in the NetBSD headers.

This is ok to push.  OOC, this is the only spot in the whole GDB code base
that has this problem?

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Rename the read symbol to xread
  2020-03-18  0:43             ` Simon Marchi
@ 2020-03-18  1:01               ` Kamil Rytarowski
  2020-03-18 20:51               ` Tom Tromey
  1 sibling, 0 replies; 10+ messages in thread
From: Kamil Rytarowski @ 2020-03-18  1:01 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 418 bytes --]

On 18.03.2020 01:43, Simon Marchi wrote:
> On 2020-03-17 7:57 p.m., Kamil Rytarowski wrote:
>> This avoids clashes with macro read in the NetBSD headers.
> 
> This is ok to push.  OOC, this is the only spot in the whole GDB code base
> that has this problem?
> 
> Simon
> 

Right now the only place that triggers this problem.

Another issue is with 'reg', but that one can be wrapped with a namespace.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Rename the read symbol to xread
  2020-03-18  0:43             ` Simon Marchi
  2020-03-18  1:01               ` Kamil Rytarowski
@ 2020-03-18 20:51               ` Tom Tromey
  1 sibling, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2020-03-18 20:51 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Kamil Rytarowski, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2020-03-17 7:57 p.m., Kamil Rytarowski wrote:
>> This avoids clashes with macro read in the NetBSD headers.

Simon> This is ok to push.  OOC, this is the only spot in the whole GDB code base
Simon> that has this problem?

There are several other "read" symbols.  I thought in C++ this sort of
thing was valid, so I guess that's a NetBSD bug of a sort.
Perhaps "#undef read" would be better overall.

Tom

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-18 20:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 12:45 [PATCH] Rename the read symbol to xread Kamil Rytarowski
2020-03-17 17:51 ` Christian Biesinger
2020-03-17 17:58   ` Kamil Rytarowski
2020-03-17 18:00     ` Christian Biesinger
2020-03-17 18:01       ` Kamil Rytarowski
2020-03-17 18:59         ` Christian Biesinger
2020-03-17 23:57           ` [PATCH v2] " Kamil Rytarowski
2020-03-18  0:43             ` Simon Marchi
2020-03-18  1:01               ` Kamil Rytarowski
2020-03-18 20:51               ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).