Re: [PATCH 16/19] imap-send: fix leaking memory in `imap_server_conf`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> We never free any of the config strings that we populate into the
> `struct imap_server_conf`. Fix this by creating a common exit path where
> we can free resources.

This is more like the previous step got rid of the anchor that made
these strings reachable, so we need to turn around to free them,
which is sort-of Meh, especially given that the leaked pieces of
memory are small and very much bounded.

The main benefit of this change is to allow us prepare on the
constness change in the other (read: API this thing uses from
elsewhere) parts of the system, which is a very worthy goal.

> While at it, drop the unused variables `imap_server_conf::name` and
> `nongit_ok`.

The removal of the .name member may be correct, but I suspect the
change to nongit_ok is a change in behaviour, and it could even be a
regression.

> -	setup_git_directory_gently(&nongit_ok);
> +	setup_git_directory_gently(NULL);

The general idea behind &nongit_ok is that

 - Usually setup_git_directory_gently() dies if NULL is passed
   instead of a pointer to &nongit_ok.  Most of the Git command
   wants to make sure they have a repository to operate on, so this
   is a reasonable default behaviour.

 - Some commands would want to work also without having any
   repository, possibly with limited capability (e.g., "git apply"
   may want to work as a better "GNU patch", but obviously it cannot
   do "git apply --binary --3way" without having the object
   database).  They tell setup_git_directory_gently() not to die
   when outside a repository by passing a pointer to &nongit_ok, and
   instead tell if we are in a repository by storing 0/1 in it.

The idea is that a command that is willing to work outside a
repository can disable selected features based on what it sees in
nongit_ok.  In the case of "imap-send", there is no such features
that it needs to special case, perhaps because everything it does is
supposed to work outside a repository?

So the short version of what worries me in this change is that we
used to be able to operate without having a repository at all, but
now we would barf if run outside a repository, no?

Thanks.