Re: [PATCH 05/19] tree_entry_interesting(): remove dependency on struct diff_options

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 13, 2010, 19:11 UTC
Message-ID: <7vbp4p1o9l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1292233616-27692-6-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

Show 10 quoted lines

> This function can be potentially used in more places than just
> tree-diff.c. "struct diff_options" does not make much sense outside
> diff_tree_sha1().
>
> While removing the use of diff_options, it also removes
> tree_entry_extract() call, which means S_ISDIR() uses the entry->mode
> directly, without being filtered by canon_mode() (called internally
> inside tree_entry_extract)
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The patch looks good, but the second paragraph above sounded unnecessarily alarming and I had to read it three times to make sure nothing fishy is going on ;-).

It bypasses tree-entry-extract call and uses fields of "entry" directly. With the change, entry->mode is used without first getting normalized with canon_mode(), but the only use of the mode information in this function is to check the type of the entry by giving it to S_ISDIR() macro, and the result does not change with or without canon_mode(), so it is Ok.