From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x330.google.com (mail-ot1-x330.google.com [IPv6:2607:f8b0:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id C12D838971A6 for ; Thu, 15 Dec 2022 19:11:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C12D838971A6 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ot1-x330.google.com with SMTP id m7-20020a9d6447000000b0066da0504b5eso37673otl.13 for ; Thu, 15 Dec 2022 11:11:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=rDd1sPfARH7rG4k04mQ3tECE0zUGURyqizVAGQOX/eg=; b=rWxsbnNFFVOfk9Yjs86NRBVP6qV7/1I6dWpHwjVNU4lH9azcKyF20EY4efsykEj6pw Lcjm7eihtuc7A8eeZb9t5FHj+hO6KK4EkBvZwNyTKs6L1XfqOo8sLbmhnu2y0Ua8Vxkg 4r5TUc7GCLvW/cuNZAqkQil/oRKXjiynY6BMNDg/sLImtazt3ajzLxPyYALVm/BBbnsf KJAYI/GTtN0CAFQtwNgRhqxkuLoW7PtECGLGBjmxZvxmNLDDIB0bhiNxai2KThTUlt1R ukm/ElXbMY7+s4VgDxxlIicv7BoKx4vq94DwtQHdKUVeXlogMDG6hm31AKIqNp7460qx Wvcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rDd1sPfARH7rG4k04mQ3tECE0zUGURyqizVAGQOX/eg=; b=RGm95hlIgIEoVlGC/KnOAoGP1CXJOj/pRIDVE5pk69CyqMkrsmvnq7gZ3VlmRIJ/6Z Dz2VC1aWb24HM+WyaJlyVgHMDzntmmysDMRt++9cr4vSOKLDasmMRJ+sJdDYGadQKizo ohpf47u552xcY4Gvgt4BbceujMBxluEoNU0bDLH+q6PD1jMdROY+nE9n/Zd0Fg6Az0F4 tQoAQ1c15lnXj2eij7VmKRn3dDUkj2fC+aFcqqa0SOtkcbzr8qlcL0nKrX65mW3IZvCZ 34ShhOC8NnzOEX7SyOC6/DS0d6hNx1bXVll1pJ1avefP5iFQsEavk4hzxqaiO1yUXPvy CKBg== X-Gm-Message-State: ANoB5pnzNw3GwPDlMKRsew4LRavM4JwzG4QypDaLAyW6TmnK5FIiE9kn +5990+mMBHWRRQBBvJ4a8odcOQYpkJfdUPmYXQQ= X-Google-Smtp-Source: AA0mqf6ugLw3HNbNFckH43BnxlsVo8SD2QKvpC1iIUa62Uvmy3+S63YKrC6PzvRL1uSI/oZ3wkqPEA== X-Received: by 2002:a05:6830:378b:b0:66e:61be:1a90 with SMTP id bi11-20020a056830378b00b0066e61be1a90mr15299260otb.23.1671131477986; Thu, 15 Dec 2022 11:11:17 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c2:f05e:14ba:6503:a761:d926? ([2804:1b3:a7c2:f05e:14ba:6503:a761:d926]) by smtp.gmail.com with ESMTPSA id g16-20020a056830309000b0066e64c59bbcsm3838488ots.6.2022.12.15.11.11.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 15 Dec 2022 11:11:17 -0800 (PST) Message-ID: <858408de-a8e2-1a14-1074-11092228c9d9@linaro.org> Date: Thu, 15 Dec 2022 16:11:14 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH v5 09/11] libio: Convert __vdprintf_internal to buffers Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: <2deb7d2cffb76dbcaf7ba051e616ccb7e275ed54.1670858473.git.fweimer@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <2deb7d2cffb76dbcaf7ba051e616ccb7e275ed54.1670858473.git.fweimer@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 12/12/22 12:23, Florian Weimer via Libc-alpha wrote: > The internal buffer size is set to 2048 bytes. This is less than > the original BUFSIZ value used by buffered_vfprintf before > the conversion, but it hopefully covers all cases where write > boundaries matter. > --- > include/printf_buffer.h | 4 ++ > libio/iovdprintf.c | 69 ++++++++++++++++++------------ > stdio-common/printf_buffer_flush.c | 4 ++ > 3 files changed, 49 insertions(+), 28 deletions(-) > > diff --git a/include/printf_buffer.h b/include/printf_buffer.h > index 47e252b52e..3d4ef1d06c 100644 > --- a/include/printf_buffer.h > +++ b/include/printf_buffer.h > @@ -49,6 +49,7 @@ enum __printf_buffer_mode > __printf_buffer_mode_sprintf_chk, > __printf_buffer_mode_to_file, > __printf_buffer_mode_asprintf, > + __printf_buffer_mode_dprintf, > __printf_buffer_mode_strfmon, > __printf_buffer_mode_fp, /* For __printf_fp_l_buffer. */ > __printf_buffer_mode_fp_to_wide, /* For __wprintf_fp_l_buffer. */ > @@ -305,6 +306,9 @@ void __printf_buffer_flush_to_file (struct __printf_buffer_to_file *) > struct __printf_buffer_asprintf; > void __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *) > attribute_hidden; > +struct __printf_buffer_dprintf; > +void __printf_buffer_flush_dprintf (struct __printf_buffer_dprintf *) > + attribute_hidden; > struct __printf_buffer_fp; > void __printf_buffer_flush_fp (struct __printf_buffer_fp *) > attribute_hidden; > diff --git a/libio/iovdprintf.c b/libio/iovdprintf.c > index 1e483868c9..f4cdd6b537 100644 > --- a/libio/iovdprintf.c > +++ b/libio/iovdprintf.c > @@ -24,41 +24,54 @@ > This exception applies to code released by its copyright holders > in files containing the exception. */ > > -#include > +#include > +#include > +#include > #include > +#include > +#include > > -int > -__vdprintf_internal (int d, const char *format, va_list arg, > - unsigned int mode_flags) > +struct __printf_buffer_dprintf > { > - struct _IO_FILE_plus tmpfil; > - struct _IO_wide_data wd; > - int done; > + struct __printf_buffer base; > + int fd; > + > + /* This should cover most of the packet-oriented file descriptors, > + where boundaries between writes could be visible to readers. */ > + char buf[2048]; > +}; I am even more inclined to move these magic buffer size constants to a specific header. Rest looks ok. > > -#ifdef _IO_MTSAFE_IO > - tmpfil.file._lock = NULL; > -#endif > - _IO_no_init (&tmpfil.file, _IO_USER_LOCK, 0, &wd, &_IO_wfile_jumps); > - _IO_JUMPS (&tmpfil) = &_IO_file_jumps; > - _IO_new_file_init_internal (&tmpfil); > - if (_IO_file_attach (&tmpfil.file, d) == NULL) > +void > +__printf_buffer_flush_dprintf (struct __printf_buffer_dprintf *buf) > +{ > + char *p = buf->buf; > + char *end = buf->base.write_ptr; > + while (p < end) > { > - _IO_un_link (&tmpfil); > - return EOF; > + ssize_t ret = TEMP_FAILURE_RETRY (write (buf->fd, p, end - p)); > + if (ret < 0) > + { > + __printf_buffer_mark_failed (&buf->base); > + return; > + } > + p += ret; > } > - tmpfil.file._flags |= _IO_DELETE_DONT_CLOSE; > - > - _IO_mask_flags (&tmpfil.file, _IO_NO_READS, > - _IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING); > - > - done = __vfprintf_internal (&tmpfil.file, format, arg, mode_flags); > - > - if (done != EOF && _IO_do_flush (&tmpfil.file) == EOF) > - done = EOF; > - > - _IO_FINISH (&tmpfil.file); > + buf->base.write_ptr = buf->buf; > +} > > - return done; > +int > +__vdprintf_internal (int d, const char *format, va_list arg, > + unsigned int mode_flags) > +{ > + struct __printf_buffer_dprintf buf; > + __printf_buffer_init (&buf.base, buf.buf, array_length (buf.buf), > + __printf_buffer_mode_dprintf); > + buf.fd = d; > + __printf_buffer (&buf.base, format, arg, mode_flags); > + if (__printf_buffer_has_failed (&buf.base)) > + return -1; > + __printf_buffer_flush_dprintf (&buf); > + return __printf_buffer_done (&buf.base); > } > > int > diff --git a/stdio-common/printf_buffer_flush.c b/stdio-common/printf_buffer_flush.c > index 14fe1b2df4..922340cc54 100644 > --- a/stdio-common/printf_buffer_flush.c > +++ b/stdio-common/printf_buffer_flush.c > @@ -28,6 +28,7 @@ > # pragma weak __printf_buffer_flush_snprintf > # pragma weak __printf_buffer_flush_to_file > # pragma weak __printf_buffer_flush_asprintf > +# pragma weak __printf_buffer_flush_dprintf > # pragma weak __printf_buffer_flush_fp > # pragma weak __printf_buffer_flush_fp_to_wide > # pragma weak __printf_buffer_flush_fphex_to_wide > @@ -53,6 +54,9 @@ __printf_buffer_do_flush (struct __printf_buffer *buf) > case __printf_buffer_mode_asprintf: > __printf_buffer_flush_asprintf ((struct __printf_buffer_asprintf *) buf); > return; > + case __printf_buffer_mode_dprintf: > + __printf_buffer_flush_dprintf ((struct __printf_buffer_dprintf *) buf); > + return; > case __printf_buffer_mode_strfmon: > __set_errno (E2BIG); > __printf_buffer_mark_failed (buf); Ok.