Hi Peff and René On 07/03/2024 19:42, René Scharfe wrote:
Am 07.03.24 um 10:26 schrieb Jeff King:diff --git a/sequencer.c b/sequencer.c index 991a2dbe96..664986e3b2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1840,7 +1840,7 @@ static int is_fixup_flag(enum todo_command command, unsigned flag) static void add_commented_lines(struct strbuf *buf, const void *str, size_t len) { const char *s = str; - while (len > 0 && s[0] == comment_line_char) { + while (starts_with_mem(s, len, comment_line_str)) { size_t count; const char *n = memchr(s, '\n', len); if (!n) @@ -2562,7 +2562,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item, /* left-trim */ bol += strspn(bol, " \t"); - if (bol == eol || *bol == '\r' || *bol == comment_line_char) { + if (bol == eol || *bol == '\r' || starts_with_mem(bol, eol - bol, comment_line_str)) {If the strspn() call is safe (which it is, as the caller expects the string to be NUL-terminated) then you could use starts_with() here and avoid the length calculation. But that would also match comment_line_str values that contain LF, which the _mem version does not > and that's better.
I agree with your analysis. I do wonder though if we should reject whitespace and control characters when parsing core.commentChar, it feels like accepting them is a bug waiting to happen. If comment_line_char starts with ' ' or '\t' that part will be eaten by the strspn() above and so starts_with_mem() wont match. Also we will never match a comment if comment_line_str contains '\n'.
Not sure why lines that start with CR are considered comment lines, though.
I think it is a lazy way of looking for an empty line ending in CR LF, it should really be
|| (bol[0] == '\r' && bol[1] == '\n') || Best Wishes Phillip
