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

["Luis Machado (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Luis Machado has posted comments on this change.

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


Patch Set 2:

(3 comments)

| --- 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:

Thanks! I'll fix this. Getting up to speed on the cpp-ness of it!

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

PS2, Line 195:

No use case in particular for a NULL tdesc, though i tend to think
having the check here prevents having to add such a check elsewhere.
We don't want gdbserver to die when we see a NULL tdesc for example.

Unless a NULL tdesc is a major error we don't want to allow. Then i
agree an assert would be good. Thoughts?

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

PS2, Line 199:

I'll fix this.

| +	    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-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 19 Nov 2019 14:29:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment