On Saturday 18 November 2017 07:10 AM, Junio C Hamano wrote:
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
@@ -40,6 +40,32 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
+ static const char *close_notice = NULL;
+
+ if (isatty(2) && !close_notice) {
If you reverse this condition to say (!close_notice && isatty(2)),
then you save an isatty() invocation each time if close_notice is
already assigned.
However, it's not clear how much benefit you gain from stashing this
away in a static variable. Premature optimization?
The variable being "static" could be (but it was done primarily
because it allowed me not to worry about freeing),
AFAIK, observing the way the variable 'close_notice' is used, I guess
you don't need to worry about freeing it up even if it wasn't "static".
That's my interpretation of the following section of the C standard,
========================
6.5.2.5 Compound literals
...
9 EXAMPLE 2
In contrast, in
void f(void)
{
int *p;
/*...*/
p = (int [2]){*p};
/*...*/
}
p is assigned the address of the first element of an array of two ints,
the first having the value previously pointed to by p and the second,
zero. The expressions in this compound literal need not be constant. The
unnamed object has automatic storage duration.
=========================
So the unnamed objects created as a consequence of the string literals
assigned to 'close_notice' should have "automatic" storage duration
which means you don't have to worry about allocating memory for them
which would make you worry about freeing it up. If I'm stating something
wrong, let me know.
BTW, I find making the variable 'close_notice' to be 'static' unwanted
as I couldn't find any piece of code that invokes 'launch_editor' more
than once within a single run. What could be the possible cases in which
'launch_editor' could be called twice ?
Should printing of close_notice be done before the error()? Otherwise,
you get this:
--- 8< ---
Launched your editor (...) ...There was a problem...
--- 8< ---
In my version with a far shorter message, I deliberately chose not
to clear the notice. We ran the editor, and we saw a problem. That
is what happened and that is what will be left on the terminal.
It might be a good thing to keep the notice but I think it would be
better to have that error message in a "new line". I'm not sure if it's
possible or not.
---
Kaartic
