On Thu, Oct 19, 2017 at 02:31:20PM -0700, Stefan Beller wrote: > > This is unlikely to ever come up in practice since our line > > boundaries generally come from calling strlen() in the first > > place. > > get_string_hash is called from > prepare_entry, which in turn is called from > add_lines_to_move_detection or mark_color_as_moved > diff_flush_patch_all_file_pairs > > that constructs the arguments in > diff_flush_patch > run_diff > run_diff_cmd > builtin_diff (part "/* Crazy xdl interfaces.. */") > xdi_diff_outf( fn_out_consume as arg!) > xdi_diff > xdl_diff > xdl_call_hunk_func > -> fn_out_consume(cb, line, len) > > xdl_call_hunk_func however uses pointer arithmetic instead > of strlen. So I think this sentence is not a good idea to put in > the commit message. > > It may not occur in practice, due to binary files detection using > NUL as a signal, but conceptually our move-colored(!) diffs > should be compatible with NULs with this patch now. Good catch. I just skimmed over all the diff_emit_* functions, which use strlen(). But at the bottom is emit_add_line(), which has a real length marker from xdiff. I agree we wouldn't see NULs in general, but this is maybe fixing "diff --color-moved -a". I dunno. It's probably not worth digging too much, since it seems like the right thing to do regardless. -Peff
