tdesc_get_features_xml() logic

tdesc_get_features_xml() logic

[Nicholas Kulikov]

Hello,

What is the idea to check the precondition `tdesc->xmltarget != NULL`
in assert while there is (the code below) conditional logic which
handles this case? Do I miss something or there is a flaw in assert
logic?

gdbserver/tdesc.cc:

const char *
tdesc_get_features_xml (const target_desc *tdesc)
{
  /* Either .xmltarget or .features is not NULL.  */
  gdb_assert (tdesc->xmltarget != NULL
              || (!tdesc->features.empty ()
                  && tdesc->arch != NULL));

  if (tdesc->xmltarget == NULL)
    {
      std::string buffer ("@");
      print_xml_feature v (&buffer);
      tdesc->accept (v);
      tdesc->xmltarget = xstrdup (buffer.c_str ());
    }

  return tdesc->xmltarget;
}

This assert triggered at first call of tdesc_get_features_xml() when
tdesc->xmltarget is NULL.

Re: tdesc_get_features_xml() logic

[Joel Brobecker]

> What is the idea to check the precondition `tdesc->xmltarget != NULL`
> in assert while there is (the code below) conditional logic which
> handles this case? Do I miss something or there is a flaw in assert
> logic?

I think the comment just above the call to gdb_assert explains it?

    /* Either .xmltarget or .features is not NULL.  */

Said differently, if you don't have one, then you have the other.

> gdbserver/tdesc.cc:
> 
> const char *
> tdesc_get_features_xml (const target_desc *tdesc)
> {
>   /* Either .xmltarget or .features is not NULL.  */
>   gdb_assert (tdesc->xmltarget != NULL
>               || (!tdesc->features.empty ()
>                   && tdesc->arch != NULL));
> 
>   if (tdesc->xmltarget == NULL)
>     {
>       std::string buffer ("@");
>       print_xml_feature v (&buffer);
>       tdesc->accept (v);
>       tdesc->xmltarget = xstrdup (buffer.c_str ());
>     }
> 
>   return tdesc->xmltarget;
> }
> 
> This assert triggered at first call of tdesc_get_features_xml() when
> tdesc->xmltarget is NULL.

-- 
Joel