> packet_read_line() calls packet_read_line_generic() that calls > packet_read() with the fixed packet_buffer[] that is sized to be > LARGE_PACKET_MAX. > > Can this check even trigger? I thought when len == LARGE_PACKET_MAX, this could trigger. Though on inspection of packet_read, we already reject packets that have size LARGE_PACKET_MAX, and the largest size allowed is LARGE_PACKET_MAX - 1. I guess we can remove it altogether and still be future proof. If we ever want to allow larger push options we either need to have larger (variable sized) packets or a capability push-options-v2, so I'll rip this check out. > >> + lf = strchr(line, '\n'); >> + if (lf) >> + *lf = '\0'; > > packet_read_line() -> packet_read_line_generic() calls packet_read() > with PACKET_READ_CHOMP_NEWLINE flag bit; do we need this check? This check was not about "option with lf at end\n", but rather we want to chop off "option\nover\nmultiple\nlines" ? Although as you remarked in another email, this would not pose a problem for the shell variable, so we could also drop it to allow multi line options. will do. > >> + >> + string_list_append(ret, line); >> + } >> + >> + return ret; >> +} > > Other than that, looks good to me. > > Thanks. -- 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