From: Junio C Hamano Date: Mon, 13 Dec 2010 19:11:34 GMT Subject: Re: [PATCH 05/19] tree_entry_interesting(): remove dependency on struct diff_options 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 writes: > 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 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.