On Thu, Jun 25, 2015 at 9:40 PM, Paul Tan <pyokagan@xxxxxxxxx> wrote: > On Wed, Jun 24, 2015 at 11:10 PM, Johannes Schindelin > <johannes.schindelin@xxxxxx> wrote: >>> + else if (l1.len && l2.len && l3.len && is_email(paths->items->string)) >>> + ret = PATCH_FORMAT_MBOX; >> >> Maybe we can do better than this by folding the `is_email() function into this here function, reusing the same strbuf to read the lines and keeping track of the email header lines we saw... I would really like to avoid opening the same file twice just to figure out whether it is in email format. > > Okay, how about every time we call a strbuf_getline(), we save the > line to a string_list as well? Like string_list_getline_crlf() below: > [...] Hmm, on second thought, I don't think it's worth the code complexity. While I agree it's desirable to not open the file twice, I don't think detecting the patch format is so IO intensive that it needs to be optimized to that extent. Instead, we should probably just modify is_email() to take a FILE*, and then fseek(fp, 0L, SEEK_SET) to the beginning. I think the logic of is_email() is complex and so it should not be folded into the detect_patch_format() function, especially since we may add detection of other patch formats in the future, and may need more complex heuristics. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html