Fix --type-id-style hash for empty internal names.

libabigail's intern_string class treats the empty string specially. It
is not safe to call the raw method without checking for a empty
pointer. It is safe to convert to std::string.

This commit changes the XML writer to convert interned strings to
std::strings before computing their hashes.

	* src/abg-writer.cc (write_context::get_id_for_type): When
	hashing internal type names, convert to std::string rather
	than using the raw method directly as this will avoid a null
	pointer dereference in the case of an empty string; tabify
	code indentation.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-writer.cc | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

On Mon, Jul 06, 2020 at 10:21:33AM +0100, Giuliano Procida wrote:
>libabigail's intern_string class treats the empty string specially. It
>is not safe to call the raw method without checking for a empty
>pointer. It is safe to convert to std::string.
>
>This commit changes the XML writer to convert interned strings to
>std::strings before computing their hashes.
>
>	* src/abg-writer.cc (write_context::get_id_for_type): When
>	hashing internal type names, convert to std::string rather
>	than using the raw method directly as this will avoid a null
>	pointer dereference in the case of an empty string; tabify
>	code indentation.
>

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-writer.cc | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
>diff --git a/src/abg-writer.cc b/src/abg-writer.cc
>index ce0bae2d..b9813ef4 100644
>--- a/src/abg-writer.cc
>+++ b/src/abg-writer.cc
>@@ -450,20 +450,21 @@ public:
>     switch (m_type_id_style)
>       {
>       case SEQUENCE_TYPE_ID_STYLE:
>-        {
>-          interned_string id = get_id_manager().get_id_with_prefix("type-id-");
>-          return m_type_id_map[c] = id;
>-        }
>+	{
>+	  interned_string id = get_id_manager().get_id_with_prefix("type-id-");
>+	  return m_type_id_map[c] = id;
>+	}
>       case HASH_TYPE_ID_STYLE:
>-        {
>-          interned_string pretty = c->get_cached_pretty_representation(true);
>-          size_t hash = hashing::fnv_hash(*pretty.raw());
>-          while (!m_used_type_id_hashes.insert(hash).second)
>-            ++hash;
>-          std::ostringstream os;
>-          os << std::hex << std::setfill('0') << std::setw(8) << hash;
>-          return m_type_id_map[c] = c->get_environment()->intern(os.str());
>-        }
>+	{
>+	  interned_string pretty = c->get_cached_pretty_representation(true);
>+	  std::string str = pretty;

could be const

>+	  size_t hash = hashing::fnv_hash(str);
>+	  while (!m_used_type_id_hashes.insert(hash).second)
>+	    ++hash;
>+	  std::ostringstream os;
>+	  os << std::hex << std::setfill('0') << std::setw(8) << hash;
>+	  return m_type_id_map[c] = c->get_environment()->intern(os.str());
>+	}
>       }
>     ABG_ASSERT_NOT_REACHED;
>     return interned_string();
>-- 
>2.27.0.212.ge8ba1cc988-goog
>

Giuliano Procida <gprocida@google.com> a écrit:

[...]

> +++ b/src/abg-writer.cc
> @@ -450,20 +450,21 @@ public:
>      switch (m_type_id_style)
>        {
>        case SEQUENCE_TYPE_ID_STYLE:
> -        {
> -          interned_string id = get_id_manager().get_id_with_prefix("type-id-");
> -          return m_type_id_map[c] = id;
> -        }
> +	{
> +	  interned_string id = get_id_manager().get_id_with_prefix("type-id-");
> +	  return m_type_id_map[c] = id;
> +	}
>        case HASH_TYPE_ID_STYLE:
> -        {
> -          interned_string pretty = c->get_cached_pretty_representation(true);
> -          size_t hash = hashing::fnv_hash(*pretty.raw());
> -          while (!m_used_type_id_hashes.insert(hash).second)
> -            ++hash;
> -          std::ostringstream os;
> -          os << std::hex << std::setfill('0') << std::setw(8) << hash;
> -          return m_type_id_map[c] = c->get_environment()->intern(os.str());
> -        }
> +	{
> +	  interned_string pretty = c->get_cached_pretty_representation(true);
> +	  std::string str = pretty;
> +	  size_t hash = hashing::fnv_hash(str);

This should work:
          size_t hash = hashing::fnv_hash(pretty);

And we'd do away with the line above:
   +	  std::string str = pretty;

[...]

> +	  while (!m_used_type_id_hashes.insert(hash).second)
> +	    ++hash;
> +	  std::ostringstream os;
> +	  os << std::hex << std::setfill('0') << std::setw(8) << hash;
> +	  return m_type_id_map[c] = c->get_environment()->intern(os.str());
> +	}
>        }
>      ABG_ASSERT_NOT_REACHED;
>      return interned_string();


Applied with the above change.

Thanks!

Cheers,