Jiang Xin <worldhello.net@xxxxxxxxx> writes:
> Add new flag "PACKET_READ_USE_SIDEBAND" for "packet_read_with_status()"
> to prevent mangling newline characters in sideband messages.
>
> Helped-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> Signed-off-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
> ---
> pkt-line.c | 32 ++++++++++++++++++++++++++++++--
> pkt-line.h | 1 +
> t/t0070-fundamental.sh | 2 +-
> 3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 5943777a17..865ad19484 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -462,8 +462,33 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
> }
>
> if ((options & PACKET_READ_CHOMP_NEWLINE) &&
> - len && buffer[len-1] == '\n')
> - len--;
> + len && buffer[len-1] == '\n') {
> + if (options & PACKET_READ_USE_SIDEBAND) {
> + int band = *buffer & 0xff;
> + switch (band) {
> + case 1:
> + /* Chomp newline for payload */
> + len--;
> + break;
> + case 2:
> + /* fallthrough */
> + case 3:
> + /*
> + * Do not chomp newline for progress and error
> + * message.
> + */
> + break;
> + default:
> + /*
> + * Bad sideband, let's leave it to
> + * demultiplex_sideband() to catch this error.
> + */
> + break;
> + }
> + } else {
> + len--;
> + }
> + }
That's a mouthful and we could shorten it a lot, but is very easy to
follow the logic ;-)
> diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
> index a927c665d6..138c2becc1 100755
> --- a/t/t0070-fundamental.sh
> +++ b/t/t0070-fundamental.sh
> @@ -97,7 +97,7 @@ test_expect_success 'unpack-sideband with demultiplex_sideband(), no chomp newli
> test_cmp expect-err err
> '
>
> -test_expect_failure 'unpack-sideband with demultiplex_sideband(), chomp newline' '
> +test_expect_success 'unpack-sideband with demultiplex_sideband(), chomp newline' '
> test_when_finished "rm -f expect-out expect-err" &&
> test-tool pkt-line send-split-sideband >split-sideband &&
> test-tool pkt-line unpack-sideband \
We cannot quite see what got fixed that is outside the postimage,
but it is nice that we have one fewer test_expect_failure in the
end.
Will queue. Thanks.
