On Wed, Aug 07, 2024 at 05:11:28PM +1000, James Liu wrote: > On Tue Aug 6, 2024 at 7:00 PM AEST, Patrick Steinhardt wrote: > > Refactor the code so that we initialize the value with another array. > > This allows us to free the value in case the string is not pointing to > > that constant array anymore. > > > > diff --git a/environment.c b/environment.c > > index 5cea2c9f54..8297c6e37b 100644 > > --- a/environment.c > > +++ b/environment.c > > @@ -113,7 +113,8 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT; > > * The character that begins a commented line in user-editable file > > * that is subject to stripspace. > > */ > > -const char *comment_line_str = "#"; > > +const char comment_line_str_default[] = "#"; > > +const char *comment_line_str = comment_line_str_default; > > int auto_comment_line_char; > > > > /* Parallel index stat data preload? */ > > Is my understanding correct that `comment_line_str` is now just a > pointer to the `comment_line_str_default` array, and thus can be freed > once we're done with it? Not quite. By default, `comment_line_str` also points to comment_line_str_default`, which is a string constant and thus neither of these variables can be free'd. But what this split allows us to do is to check whether `comment_line_str` has changed from the default, and thus we can conditionall free it when it does not point to the default value anymore. Now that I revisit this commit I'm not quite happy with it anymore. We still need to have the cast, which is somewhat awkward. I think the better solution is to instead have a `comment_line_str_allocated` variable that is non-constant. I'll adapt the code accordingly. An even better solution would be to have `struct strbuf` provide an initializer that populates it with a string constant. But that feels like a larger undertaking, so I'll leave that for the future. Patrick
Attachment:
signature.asc
Description: PGP signature
