Jeff King <p...@peff.net> writes: > I wondered if we might also leak when seeing duplicate config options > (i.e., leaking the old one when replacing it with the new). But we don't > actually strdup() the configured remote names, but instead just point > into the "struct branch", which owns the data.
In addition, we do not copy this string to remote->name in make_remote(), so even if we start allowing destruction of existing remote, the resulting code will stay safe. > So I think an even better fix would be: > > -- >8 -- > Subject: remote: do not copy "origin" string literal > > Our default_remote_name starts at "origin", but may be > overridden by the config file. In the former case, we > allocate a new string, but in the latter case, we point to > the remote name in an existing "struct branch". > > This gives the variable inconsistent free() semantics (we > are sometimes responsible for freeing the string and > sometimes pointing to somebody else's storage), and causes a > small leak when the allocated string is overridden by > config. > > We can fix both by simply dropping the extra copy and > pointing to the string literal. > > Noticed-by: Felipe Contreras <felipe.contre...@gmail.com> > Signed-off-by: Jeff King <p...@peff.net> Sounds sensible. Thanks. > --- > remote.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/remote.c b/remote.c > index e9fedfa..9f1a8aa 100644 > --- a/remote.c > +++ b/remote.c > @@ -483,7 +483,7 @@ static void read_config(void) > int flag; > if (default_remote_name) /* did this already */ > return; > - default_remote_name = xstrdup("origin"); > + default_remote_name = "origin"; > current_branch = NULL; > head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag); > if (head_ref && (flag & REF_ISSYMREF) && -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html