From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22b.google.com (mail-oi1-x22b.google.com [IPv6:2607:f8b0:4864:20::22b]) by sourceware.org (Postfix) with ESMTPS id 4EC5A3896C31 for ; Thu, 15 Dec 2022 19:14:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4EC5A3896C31 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-oi1-x22b.google.com with SMTP id t62so8432oib.12 for ; Thu, 15 Dec 2022 11:14:47 -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=QdTkwRx9SgUy5D4ewjFWcux+9/UcFIZDd8Lm1BL5LzA=; b=HpU3JR/Bh0q92A3lLxfMhdBS99woVHLELuK6n1Gt006a4FUU46C71BCWH2vgwIJ4b2 lSHps5bJNg1wnpkQFCf9xD0BMNfUtkavnaQL0uuPlLzTa0W5ibFW4kXTW/rtPjclU09a HLw54msOb8NyYsk5WVsiD0Zh7KP34XvuP8G93SMEf0X4FifIhJt1T5P1ZgHN+hfKCTO7 2TBMLA+9/PsVAHoBVcPtwlzCJ10o+tIWdBG1klV3H/41idr5z+T2nEe08PXG1Vt/tFR1 gIytrjmbT9XXfSmquHTFlmTeeMaFk31xWXbquomL6Rhu7Tp3Su7R5nbvHjm0BY9bj+gE Fx1A== 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=QdTkwRx9SgUy5D4ewjFWcux+9/UcFIZDd8Lm1BL5LzA=; b=REZDNj5ahTwIJxYrsIxUIvf+1M/wtqe7Ur59AieV/Qb63YfERMKqJEWHV2QLsmjS5w YdDQXzLuiYNgL9ff/jAS2kmMkgQnkWGXgktdEhEyctrkLqwpP4QPolZzk9PD//DNNJVT RxUsGxyfRLQR43zkX0Ygq7fECvD85r5KcoYOde0tng8QHqja/c6wCRsgSZMaUTUzEydV c9kI4Ms49MRV0PucX0KIp63I5KbXpTAoiTA+MHyik1WALcU0yy86eILD6YbQyud78vfj nZck6NO6xoZ9txfcNBbIIQmXOVCnl4yh79Be2bXXImE5wNgM0hTZ7yEFPvAy/vfcE5ZS YmcQ== X-Gm-Message-State: ANoB5pk+1WxkflbgVPqA0FkI5Tm8aY/5pILk2wLrtTjGgBMtuTKJtAdz rC2Psp0rmrfwWR1AST9Uqockz1ab3YwGRkkNzFI= X-Google-Smtp-Source: AA0mqf6eXdQWuf5Lt+d6PAtoIyMkHymUyHpZe+twaiGOYwr69ES4pTpnFJemmY8Znrf+c3XHqyGxPA== X-Received: by 2002:a05:6808:23ce:b0:35b:9147:395d with SMTP id bq14-20020a05680823ce00b0035b9147395dmr17393478oib.33.1671131686523; Thu, 15 Dec 2022 11:14:46 -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 m21-20020a0568080f1500b003539686cb7bsm1359299oiw.53.2022.12.15.11.14.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 15 Dec 2022 11:14:45 -0800 (PST) Message-ID: <9ab5f463-fe9c-8c11-7b83-d9e330bc0275@linaro.org> Date: Thu, 15 Dec 2022 16:14:43 -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 10/11] libio: Convert __obstack_vprintf_internal to buffers (bug 27124) Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: <45234c5d6ba9556451424940fa746e9e66b1a6f4.1670858473.git.fweimer@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <45234c5d6ba9556451424940fa746e9e66b1a6f4.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,KAM_SHORT,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:24, Florian Weimer via Libc-alpha wrote: > This fixes bug 27124 because the problematic built-in vtable is gone. Patch looks good, some minor nit below. > --- > include/printf_buffer.h | 4 + > libio/obprintf.c | 170 +++++++++-------------------- > stdio-common/printf_buffer_flush.c | 4 + > 3 files changed, 58 insertions(+), 120 deletions(-) > > diff --git a/include/printf_buffer.h b/include/printf_buffer.h > index 3d4ef1d06c..a8211dd657 100644 > --- a/include/printf_buffer.h > +++ b/include/printf_buffer.h > @@ -54,6 +54,7 @@ enum __printf_buffer_mode > __printf_buffer_mode_fp, /* For __printf_fp_l_buffer. */ > __printf_buffer_mode_fp_to_wide, /* For __wprintf_fp_l_buffer. */ > __printf_buffer_mode_fphex_to_wide, /* For __wprintf_fphex_l_buffer. */ > + __printf_buffer_mode_obstack, Maybe add 'For __printf_buffer_flush_obstack'? > }; > > /* Buffer for fast character writing with overflow handling. > @@ -319,6 +320,9 @@ struct __printf_buffer_fphex_to_wide; > void __printf_buffer_flush_fphex_to_wide (struct > __printf_buffer_fphex_to_wide *) > attribute_hidden; > +struct __printf_buffer_obstack; > +void __printf_buffer_flush_obstack (struct __printf_buffer_obstack *) > + attribute_hidden; > > struct __wprintf_buffer_to_file; > void __wprintf_buffer_flush_to_file (struct __wprintf_buffer_to_file *) > diff --git a/libio/obprintf.c b/libio/obprintf.c > index c220be9adc..459d73ebf3 100644 > --- a/libio/obprintf.c > +++ b/libio/obprintf.c > @@ -16,131 +16,59 @@ > License along with the GNU C Library; if not, see > . */ > > - > -#include > -#include "libioP.h" > -#include "strfile.h" > #include > -#include > -#include > +#include > #include > +#include > #include > -#include > - > +#include > > -struct _IO_obstack_file > +struct __printf_buffer_obstack > { > - struct _IO_FILE_plus file; > + struct __printf_buffer base; > struct obstack *obstack; > -}; > - > - > -static int > -_IO_obstack_overflow (FILE *fp, int c) > -{ > - struct obstack *obstack = ((struct _IO_obstack_file *) fp)->obstack; > - int size; > - > - /* Make room for another character. This might as well allocate a > - new chunk a memory and moves the old contents over. */ > - assert (c != EOF); > - obstack_1grow (obstack, c); > - > - /* Setup the buffer pointers again. */ > - fp->_IO_write_base = obstack_base (obstack); > - fp->_IO_write_ptr = obstack_next_free (obstack); > - size = obstack_room (obstack); > - fp->_IO_write_end = fp->_IO_write_ptr + size; > - /* Now allocate the rest of the current chunk. */ > - obstack_blank_fast (obstack, size); > - > - return c; > -} > > + /* obstack_1grow is called for compatibility reasons. This needs > + one extra character, and this is the backing store for it. */ > + char ch; > +}; > > -static size_t > -_IO_obstack_xsputn (FILE *fp, const void *data, size_t n) > +void > +__printf_buffer_flush_obstack (struct __printf_buffer_obstack *buf) > { > - struct obstack *obstack = ((struct _IO_obstack_file *) fp)->obstack; > + /* About to switch buffers, so record the bytes written so far. */ > + buf->base.written += buf->base.write_ptr - buf->base.write_base; > > - if (fp->_IO_write_ptr + n > fp->_IO_write_end) > + if (buf->base.write_ptr == &buf->ch + 1) > { > - int size; > - > - /* We need some more memory. First shrink the buffer to the > - space we really currently need. */ > - obstack_blank_fast (obstack, fp->_IO_write_ptr - fp->_IO_write_end); > - > - /* Now grow for N bytes, and put the data there. */ > - obstack_grow (obstack, data, n); > - > - /* Setup the buffer pointers again. */ > - fp->_IO_write_base = obstack_base (obstack); > - fp->_IO_write_ptr = obstack_next_free (obstack); > - size = obstack_room (obstack); > - fp->_IO_write_end = fp->_IO_write_ptr + size; > - /* Now allocate the rest of the current chunk. */ > - obstack_blank_fast (obstack, size); > + /* Errors are reported via a callback mechanism (presumably for > + process termination). */ > + obstack_1grow (buf->obstack, buf->ch); > + buf->base.write_base = obstack_next_free (buf->obstack); > + buf->base.write_ptr = buf->base.write_base; > + size_t size = obstack_room (buf->obstack); > + buf->base.write_end = buf->base.write_ptr + size; > + /* Reserve the space on the obstack size. */ > + obstack_blank_fast (buf->obstack, size); > } > else > - fp->_IO_write_ptr = __mempcpy (fp->_IO_write_ptr, data, n); > - > - return n; > + { > + /* Obtain the extra character. */ > + buf->base.write_base = &buf->ch; > + buf->base.write_ptr = &buf->ch; > + buf->base.write_end = &buf->ch + 1; > + } > } > > - > -/* the jump table. */ > -const struct _IO_jump_t _IO_obstack_jumps libio_vtable attribute_hidden = > -{ > - JUMP_INIT_DUMMY, > - JUMP_INIT(finish, NULL), > - JUMP_INIT(overflow, _IO_obstack_overflow), > - JUMP_INIT(underflow, NULL), > - JUMP_INIT(uflow, NULL), > - JUMP_INIT(pbackfail, NULL), > - JUMP_INIT(xsputn, _IO_obstack_xsputn), > - JUMP_INIT(xsgetn, NULL), > - JUMP_INIT(seekoff, NULL), > - JUMP_INIT(seekpos, NULL), > - JUMP_INIT(setbuf, NULL), > - JUMP_INIT(sync, NULL), > - JUMP_INIT(doallocate, NULL), > - JUMP_INIT(read, NULL), > - JUMP_INIT(write, NULL), > - JUMP_INIT(seek, NULL), > - JUMP_INIT(close, NULL), > - JUMP_INIT(stat, NULL), > - JUMP_INIT(showmanyc, NULL), > - JUMP_INIT(imbue, NULL) > -}; > - > - Remove _IO_obstack_jumps from tst-relro-libc.out list. > int > __obstack_vprintf_internal (struct obstack *obstack, const char *format, > va_list args, unsigned int mode_flags) > { > - struct obstack_FILE > - { > - struct _IO_obstack_file ofile; > - } new_f; > - int result; > - int size; > - int room; > - > -#ifdef _IO_MTSAFE_IO > - new_f.ofile.file.file._lock = NULL; > -#endif > - > - _IO_no_init (&new_f.ofile.file.file, _IO_USER_LOCK, -1, NULL, NULL); > - _IO_JUMPS (&new_f.ofile.file) = &_IO_obstack_jumps; > - room = obstack_room (obstack); > - size = obstack_object_size (obstack) + room; > + /* Legacy setup code for compatibility. */ > + size_t room = obstack_room (obstack); > + size_t size = obstack_object_size (obstack) + room; > if (size == 0) > { > - /* We have to handle the allocation a bit different since the > - `_IO_str_init_static' function would handle a size of zero > - different from what we expect. */ > - > /* Get more memory. */ > obstack_make_room (obstack, 64); > > @@ -151,27 +79,29 @@ __obstack_vprintf_internal (struct obstack *obstack, const char *format, > assert (size != 0); > } > > - _IO_str_init_static_internal ((struct _IO_strfile_ *) &new_f.ofile, > - obstack_base (obstack), > - size, obstack_next_free (obstack)); > + struct __printf_buffer_obstack buf; > + { > + /* The obstack write location might be in the middle of an object. */ > + char *ptr = obstack_next_free (obstack); > + char *end = obstack_base (obstack) + size; > + __printf_buffer_init (&buf.base, ptr, end - ptr, > + __printf_buffer_mode_obstack); > + } > + buf.obstack = obstack; > + > /* Now allocate the rest of the current chunk. */ > - assert (size == (new_f.ofile.file.file._IO_write_end > - - new_f.ofile.file.file._IO_write_base)); > - assert (new_f.ofile.file.file._IO_write_ptr > - == (new_f.ofile.file.file._IO_write_base > - + obstack_object_size (obstack))); > obstack_blank_fast (obstack, room); > > - new_f.ofile.obstack = obstack; > - > - result = __vfprintf_internal (&new_f.ofile.file.file, format, args, > - mode_flags); > + __printf_buffer (&buf.base, format, args, mode_flags); > > - /* Shrink the buffer to the space we really currently need. */ > - obstack_blank_fast (obstack, (new_f.ofile.file.file._IO_write_ptr > - - new_f.ofile.file.file._IO_write_end)); > + if (buf.base.write_ptr == &buf.ch + 1) > + /* buf.ch is in use. Put it into the obstack. */ > + obstack_1grow (buf.obstack, buf.ch); > + else if (buf.base.write_ptr != &buf.ch) > + /* Shrink the buffer to the space we really currently need. */ > + obstack_blank_fast (buf.obstack, buf.base.write_ptr - buf.base.write_end); > > - return result; > + return __printf_buffer_done (&buf.base); > } > > int Ok. > diff --git a/stdio-common/printf_buffer_flush.c b/stdio-common/printf_buffer_flush.c > index 922340cc54..42afe49413 100644 > --- a/stdio-common/printf_buffer_flush.c > +++ b/stdio-common/printf_buffer_flush.c > @@ -32,6 +32,7 @@ > # pragma weak __printf_buffer_flush_fp > # pragma weak __printf_buffer_flush_fp_to_wide > # pragma weak __printf_buffer_flush_fphex_to_wide > +# pragma weak __printf_buffer_flush_obstack > #endif /* !SHARED */ > > static void > @@ -72,6 +73,9 @@ __printf_buffer_do_flush (struct __printf_buffer *buf) > __printf_buffer_flush_fphex_to_wide > ((struct __printf_buffer_fphex_to_wide *) buf); > return; > + case __printf_buffer_mode_obstack: > + __printf_buffer_flush_obstack ((struct __printf_buffer_obstack *) buf); > + return; > } > __builtin_trap (); > }