Re: [PATCH 5b/6] diagnostics: Remove null-termination requirement for json::string

[David Malcolm <dmalcolm@redhat.com>]

On Fri, 2022-11-04 at 17:05 -0400, Lewis Hyatt wrote:
> [PATCH 5b/6] diagnostics: Remove null-termination requirement for
> json::string
> 
> json::string currently handles null-terminated data and so can't work
> with
> data that may contain embedded null bytes or that is not null-
> terminated.
> Supporting such data will make json::string more robust in some
> contexts, such
> as SARIF output, which uses it to output user source code that may
> contain
> embedded null bytes.
> 
> gcc/ChangeLog:
> 
> 	* json.h (class string): Add M_LEN member to store the
> length of
> 	the data.  Add constructor taking an explicit length.
> 	* json.cc (string::string):  Implement the new constructor.
> 	(string::print): Support print strings that are not null-
> terminated.
> 	Escape embdedded null bytes on output.
> 	(test_writing_strings): Test the new null-byte-related
> features of
> 	json::string.
> 

[...snip...]

> diff --git a/gcc/json.h b/gcc/json.h
> index f272981259b..f7afd843dc5 100644
> --- a/gcc/json.h
> +++ b/gcc/json.h
> @@ -156,16 +156,19 @@ class integer_number : public value
>  class string : public value
>  {
>   public:
> -  string (const char *utf8);
> +  explicit string (const char *utf8);
> +  string (const char *utf8, size_t len);
>    ~string () { free (m_utf8); }
>  
>    enum kind get_kind () const final override { return JSON_STRING; }
>    void print (pretty_printer *pp) const final override;
>  
>    const char *get_string () const { return m_utf8; }

I worried that json::string::get_string previously returned a NUL-
terminated string, but now there's no guarantee of termination, and
that this might break something.  But I checked, and it seems that this
accessor doesn't get used anywhere in our source tree.

> +  size_t get_length () const { return m_len; }

Does anything actually use this?

Perhaps it might make sense to delete the get_string accessor, and if
we ever need one, replace it with an accessor that returns a char_span?

>  
>   private:
>    char *m_utf8;
> +  size_t m_len;
>  };
>  

Thanks for adding the unit test.

The 5b patch is OK for trunk.

Dave