This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] libio: Multiple fixes for open_{w}memstram (BZ#18241 and BZ#20181)
On 05/08/2016 17:11, Andreas Schwab wrote:
> On Fr, Aug 05 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>
>> For invalid position a different return error than EOF is used so
>> _IO_seekoff_unlocked can set errno accordingly.
>
> Is that needed? Can't the functions set errno themselves?
I don't have a strong preference here, I just noted that other strops.c function
do not touched errno and I though to be safer to set on _IO_seekoff function.
But I noted that fileops.c does set errno, so I thunk we can do it here as
well.
>
>> diff --git a/libio/tst-memstream3.c b/libio/tst-memstream3.c
>> new file mode 100644
>> index 0000000..ce4e60d
>> --- /dev/null
>> +++ b/libio/tst-memstream3.c
>> @@ -0,0 +1,155 @@
>> +/* Test for open_memstream implementation.
>> + Copyright (C) 2016 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <http://www.gnu.org/licenses/>. */
>> +
>> +#include <mcheck.h>
>> +#include <stdio.h>
>> +#include <errno.h>
>> +
>> +
>> +#ifndef CHAR_T
>> +# define CHAR_T char
>> +# define W(o) o
>> +# define OPEN_MEMSTREAM open_memstream
>> +# define PRINTF printf
>> +# define FWRITE fwrite
>> +# define FPUTC fputc
>> +# define STRCMP strcmp
>> +#endif
>> +
>> +#define S(s) S1 (s)
>> +#define S1(s) #s
>> +
>> +static void
>> +mcheck_abort (enum mcheck_status ev)
>> +{
>> + printf ("mecheck failed with status %d\n", (int) ev);
>> + exit (1);
>> +}
>> +
>> +#define LOC2(l) "error: " __FILE__ ":" #l
>> +#define LOC1(l) LOC2(l)
>> +#define ERROR_RET1(...) \
>> + { printf(LOC1(__LINE__) ": " __VA_ARGS__); return 1; }
>
> Putting __FILE__ into the format string is unsafe, it could contain %.
>
>> @@ -262,25 +279,29 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>> /* Move the get pointer, if requested. */
>> if (mode & _IOS_INPUT)
>> {
>> + _IO_ssize_t base;
>> switch (dir)
>> {
>> - case _IO_seek_end:
>> - offset += cur_size;
>> + case _IO_seek_set:
>> + base = 0;
>> break;
>> case _IO_seek_cur:
>> - offset += (fp->_wide_data->_IO_read_ptr
>> + base = (fp->_wide_data->_IO_read_ptr
>> - fp->_wide_data->_IO_read_base);
>
> Reindent.
Ack.
>
>> break;
>> - default: /* case _IO_seek_set: */
>> + default: /* case _IO_seek_end: */
>> + base = cur_size;
>> break;
>> }
>> - if (offset < 0)
>> - return EOF;
>> - if ((_IO_ssize_t) offset > cur_size
>> - && enlarge_userbuf (fp, offset, 1) != 0)
>> + _IO_ssize_t maxval = SSIZE_MAX/sizeof(wchar_t) - base;
>
> Space around operator.
Ack.
>
>> @@ -289,26 +310,30 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>> /* Move the put pointer, if requested. */
>> if (mode & _IOS_OUTPUT)
>> {
>> + _IO_ssize_t base;
>> switch (dir)
>> {
>> - case _IO_seek_end:
>> - offset += cur_size;
>> + case _IO_seek_set:
>> + base = 0;
>> break;
>> case _IO_seek_cur:
>> - offset += (fp->_wide_data->_IO_write_ptr
>> + base = (fp->_wide_data->_IO_write_ptr
>> - fp->_wide_data->_IO_write_base);
>
> Reindent.
>
Ack.
>> break;
>> - default: /* case _IO_seek_set: */
>> + default: /* case _IO_seek_end: */
>> + base = cur_size;
>> break;
>> }
>> - if (offset < 0)
>> - return EOF;
>> - if ((_IO_ssize_t) offset > cur_size
>> - && enlarge_userbuf (fp, offset, 0) != 0)
>> + _IO_ssize_t maxval = SSIZE_MAX/sizeof(wchar_t) - base;
>
> Space around operator.
>
Ack.
> Andreas.
>