This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 2/3] Add an optional "alias" attribute to syscall entries.
On 11/9/18 8:31 AM, Pedro Alves wrote:
> Hi,
>
> On 11/06/2018 05:54 PM, John Baldwin wrote:
>
>> --- a/gdb/syscalls/gdb-syscalls.dtd
>> +++ b/gdb/syscalls/gdb-syscalls.dtd
>> @@ -12,4 +12,5 @@
>> <!ATTLIST syscall
>> name CDATA #REQUIRED
>> number CDATA #REQUIRED
>> + alias CDATA #IMPLIED
>> groups CDATA #IMPLIED>
>
> I think there should be a NEWS entry for this.
Ok.
> I'd think there should be a docs update as well, but it
> looks like this dtd isn't documented in the manual. :-/
>
>
>> + /* We have a name. Let's check if it's valid and fetch a
>> + list of matching numbers. */
>> + std::vector<int> numbers = get_syscalls_by_name (gdbarch, cur_name);
>>
>> - if (s.number == UNKNOWN_SYSCALL)
>> + if (numbers.empty ())
>> /* Here we have to issue an error instead of a warning,
>> because GDB cannot do anything useful if there's no
>> syscall number to be caught. */
>> error (_("Unknown syscall name '%s'."), cur_name);
>>
>> /* Ok, it's valid. */
>> - result.push_back (s.number);
>> + for (int number : numbers)
>> + result.push_back (number);
>
> Nit: this return-vector interface seems like leads to a bit of unnecessary
> work, and double the number of necessary vector/heap allocations -- if
> get_syscalls_by_name were passed RESULT instead of returning a new vector,
> it could add to RESULT directly? The code above would like something like this
> instead with get_syscalls_by_name returning true/false to indicate
> whether something was added:
>
> /* We have a name. Let's check if it's valid and fetch a
> list of matching numbers. */
> if (!get_syscalls_by_name (gdbarch, cur_name, result))
> /* Here we have to issue an error instead of a warning,
> because GDB cannot do anything useful if there's no
> syscall number to be caught. */
> error (_("Unknown syscall name '%s'."), cur_name);
>
> That means there's no need to copy numbers between the vectors
> (since there's only one vector).
>
> Same for get_syscalls_by_group.
Oh, yes, that is a much better idea. I will adopt that change in both
patches.
> Note that instead of:
>
>> + for (int number : numbers)
>> + result.push_back (number);
>
> You could write:
>
> result.insert (result.end (), numbers.begin (), numbers.end ());
>
> which may be more efficient assuming insert is smart enough
> to call std::vector::reserve to allocate space in one go.
Ok. I wasn't quite sure if there was a more efficient idiom to use. It's
a bit of a shame std::vector<> doesn't have an explicit append/join method
similar to list.append() in Python.
--
John Baldwin