Re: [PATCH v7 03/15] bugreport: add tool to generate debugging info

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> +int cmd_main(int argc, const char **argv)
>> +{
>> +...
>> +	if (report == NULL) {
>> +		strbuf_release(&report_path);
>> +		die("couldn't open '%s' for writing", report_path.buf);
>> +	}
>> +
>> +	strbuf_write(&buffer, report);
>> +	fclose(report);
>> +
>> +	fprintf(stderr, _("Created new report at '%s'.\n"), report_path.buf);
>> +
>> +	UNLEAK(buffer);
>> +	UNLEAK(report_path);
>> +	return -launch_editor(report_path.buf, NULL, NULL);
>
> This would be the first time (at least that _I_ know of) that we use `-`
> in this way. We seem to use `!!` a lot more often. And now I wonder
> whether there is a reason for that `-` that I missed?

In general, our preferred way to report an error from API functions
is to return a negative number and a success is reported by
returning zero.

The argument to exit(3), which the return value from the main()
function essentially is, on the other hand, is expected to be a
small non-negative integer.  As long as we are in tight control of
the range of the returned value from launch_editor() (i.e. it must
return a small non-positive integer whose negation is suitable to be
fed to exit(3)), the above is fine.

The idiom "return !!fn();" is to canonicalize the value to 0 or 1
for the caller who asked "so, did fn() give us 0 or non-zero?",
which can also be used as the value given to exit(3), and could be
more appropriate under some conditions:

 - we MUST know launch_editor() returns zero if and only if it is
   successful.

 - we do not have to be confident to be in tight control of the
   range of the returned value from launch_editor() (e.g. it could
   return a positive upon an error).

 - we MUST NOT care to differenciate different error codes returned
   from launch_editor().  IOW, we must be fine to give the invoker
   of the program only 0 (success) or 1 (unspecified failure).

Use of "return !!fn();" that is not to give to exit(3) is probably
more common in our codebase.  See apply.c::try_create_file() and the
comment before the function about its possible return values for
example.