Re: [PATCH v2 3/4] strbuf_setlen: don't write to strbuf_slopbuf

Brandon Casey [off-list ref] writes:

So is there any reason why didn't do something like the following in
the first place?

My guess is that we didn't bother; if we cared, we would have used a
single instance of const char in a read-only segment, instead of
such a macro.

On Wed, Aug 23, 2017 at 2:04 PM, Junio C Hamano [off-list ref] wrote:

Brandon Casey [off-list ref] writes:

quoted

So is there any reason why didn't do something like the following in
the first place?

My guess is that we didn't bother; if we cared, we would have used a
single instance of const char in a read-only segment, instead of
such a macro.

I think you mean something like this:

   const char * const strbuf_slopbuf = "";

..with or without "const" at the beginning.  We can't use an actual
variable like that since we also want to be able to do initialization
like:

   struct strbuf b = STRBUF_INIT;

i.e.

   struct strbuf b = { 0, 0, strbuf_slopbuf };

So the compiler needs to be able to determine that everything within
the curly braces is constant and apparently gcc cannot.


On a related note... I was just looking at object.c which also uses a
slopbuf.  We could similarly protect it from inadvertent modification
by doing something like this:

diff --git a/object.c b/object.c
index 321d7e9..4c7a041 100644
--- a/object.c
+++ b/object.c

@@ -303,7 +303,7 @@ int object_list_contains(struct object_list *list, struct ob

ject *obj)
  * A zero-length string to which object_array_entry::name can be
  * initialized without requiring a malloc/free.
  */
-static char object_array_slopbuf[1];
+static const char * const object_array_slopbuf = "";

 void add_object_array_with_path(struct object *obj, const char *name,
                                struct object_array *array,

@@ -326,7 +326,7 @@ void add_object_array_with_path(struct object

*obj, const char *name,
                entry->name = NULL;
        else if (!*name)
                /* Use our own empty string instead of allocating one: */
-               entry->name = object_array_slopbuf;
+               entry->name = (char*) object_array_slopbuf;
        else
                entry->name = xstrdup(name);
        entry->mode = mode;

On Wed, Aug 23, 2017 at 2:20 PM, Brandon Casey [off-list ref] wrote:

On Wed, Aug 23, 2017 at 2:04 PM, Junio C Hamano [off-list ref] wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

So is there any reason why didn't do something like the following in
the first place?

My guess is that we didn't bother; if we cared, we would have used a
single instance of const char in a read-only segment, instead of
such a macro.

I think you mean something like this:

   const char * const strbuf_slopbuf = "";

Ah, you probably meant something like this:

   const char strbuf_slopbuf = '\0';

which gcc will apparently place in the read-only segment.  I did not know that.

And assignment and initialization would look like:

   sb->buf = (char*) &strbuf_slopbuf;

and

   #define STRBUF_INIT  { .alloc = 0, .len = 0, .buf = (char*) &strbuf_slopbuf }

respectively.  Yeah, that's definitely preferable to a macro.
Something similar could be done in object.c.

-Brandon

On Wed, Aug 23, 2017 at 2:54 PM, Brandon Casey [off-list ref] wrote:

On Wed, Aug 23, 2017 at 2:20 PM, Brandon Casey [off-list ref] wrote:

quoted

On Wed, Aug 23, 2017 at 2:04 PM, Junio C Hamano [off-list ref] wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

So is there any reason why didn't do something like the following in
the first place?

My guess is that we didn't bother; if we cared, we would have used a
single instance of const char in a read-only segment, instead of
such a macro.

I think you mean something like this:

   const char * const strbuf_slopbuf = "";

Hmm, apparently it is sufficient to mark our current strbuf_slopbuf
array as const and initialize it with a static string to trigger its
placement into the read-only section by gcc (and clang).

   const char strbuf_slopbuf[1] = "";

-Brandon