This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]