On 09.12.2021 17:58, Pedro Martelletto wrote:
On Thu, Dec 9, 2021 at 5:33 PM Fabian Stelzer <fs@xxxxxxxxxxxx> wrote:On 06.12.2021 10:06, Pedro Martelletto wrote: >On Sun, Dec 5, 2021 at 6:50 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> So instead of the posted patch, we should do something along this >> line instead? >> >> trust_size = strcspn(line, "\n"); /* truncate at LF */ >> if (trust_size && line[trust_size - 1] == '\r') >> trust_size--; /* the LF was part of CRLF at the end */ >> >> >> >I agree that's a more consistent fix. A minor nit: if the intention is to >only trim CR as part of a CRLF sequence, we need to ensure a LF is found: > This shouldn't be necessary as we split/loop by LF just above. for (line = ssh_principals_out.buf; *line; line = strchrnul(line + 1, '\n')) { while (*line == '\n') line++; if (!*line) break; trust_size = strcspn(line, "\n"); principal = xmemdupz(line, trust_size);The loop ensures that 'line' points to the first character of ssh_principals_out.buf or to a non-NUL character after a '\n'. It does not ensure that that 'line' contains a '\n', e.g: "principalA\nprincipalB\nprincipalC\r" or just "principalA\r".
Just saw that this is still open. @pedro: do you want to send an updated version of your patch or would you like me to pick this up and send one?
