[review v2] [AArch64, SVE] Improve target description check for SVE in gdbserver

["Simon Marchi (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/690
......................................................................


Patch Set 2:

(3 comments)

Here are some minor comments.  I think Alan Hayward will be in a better position to assess the correctness of the patch.

| --- gdb/gdbserver/tdesc.c
| +++ gdb/gdbserver/tdesc.c
| @@ -184,5 +184,22 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name)
|  {
|    struct tdesc_feature *new_feature = new tdesc_feature (name);
|    tdesc->features.emplace_back (new_feature);
|    return new_feature;
|  }
| +
| +/* See gdbsupport/tdesc.h.  */
| +
| +bool
| +tdesc_contains_feature (const target_desc *tdesc, const std::string feature)

PS2, Line 193:

Pass feature as a reference.

| +{
| +  if (tdesc && !tdesc->features.empty ())

PS2, Line 195:

tdesc != nullptr

Or just assume/assert it is not NULL?  Is there a legitimate use case
for passing a NULL tdesc?

| +    {
| +      for (const tdesc_feature_up &f : tdesc->features)
| +	{
| +	  if (f->name.compare (feature) == 0)

PS2, Line 199:

This can be

 f->name == feature

| +	    return true;
| +	}
| +    }
| +
| +  return false;
| +}

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I28b782cb1677560ca9a06a1be442974b25aabae4
Gerrit-Change-Number: 690
Gerrit-PatchSet: 2
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 19 Nov 2019 14:22:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment