Re: [PATCH v4] diff: handle --no-abbrev in no-index case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/12/16 03:53 PM, Junio C Hamano wrote:
Jack Bates <bk874k@xxxxxxxxxxxxxxxx> writes:
@@ -3364,6 +3365,7 @@ void diff_setup(struct diff_options *options)

 	options->file = stdout;

+	options->abbrev = DEFAULT_ABBREV;

This is a new change relative to your earlier one.

I looked at all the callers of diff_setup() and noticed that many of
them were initializing "struct diff_options" that is on-stack that
is totally uninitialized, which means they were using a completely
random value that happened to be on the stack.

Which was surprising and made me wonder how the entire "diff" code
could have ever worked correctly for the past 10 years, as it's not
like all the users always passed --[no-]abbrev[=<value>] from the
command line.

In any case, this cannot possibly be introducing a regression; these
callsites of diff_setup() were starting from a random garbage---now
they start with -1 in this field.  If they were doing the right
thing by assigning their own abbrev to the field after diff_setup()
returned, they will continue to do the same, and otherwise they will
keep doing whatever random things they have been doing when the
uninitialized field happened to contain -1 the same way.

I didn't look carefully at the additional tests, but the code change
looks good.

Thanks.

Great, thanks for reviewing it!



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]