From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26599 invoked by alias); 5 Aug 2016 21:00:00 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 26590 invoked by uid 89); 5 Aug 2016 20:59:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=safer, adhemervalzanellalinaroorg, U*adhemerval.zanella, adhemerval.zanella@linaro.org X-HELO: mail-yw0-f172.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=z6MeDcT9H/ySwiViYySw6AZE2IIG2sZ+ncYSlT3tzIA=; b=CJAkdyYdoud5tE6wg2p+yqMCUMmHsAEQu6lpiWRCPU2KhljtbPsrfAum+acGd8s04P PqJy03HY/XJV8wV1tsk8qYbLt5sFa5FzFkwKLhU81zDdWqp8fR8KHVljnzZPIT7cItzw 0Bon6eS21fwgw0AjhICcKv79/VZ6X3PNM0ibp4CV1BAYKbZc8DuNutWj+E5ufOQ/Pj9a NWkvO7qOOMV2pOj+Tmq1XxzbkX2qjxXDwRZsBOHaWL/2BSgmBKcQxXl6f9G1z8B+iDtc jP+qB29ruh1zljBRSYiyBqkJZ/u7oFrpIyQvKymAR86G8B9E5GVs7/KTZNF7jPkL5uME Axyg== X-Gm-Message-State: AEkoouucSJiX0C9XlfjVz4p6EMygZD1gGoxyrRGhTJMH7DntgkTu2BFIP1bFWEariP8UcM5I X-Received: by 10.129.110.7 with SMTP id j7mr64601854ywc.62.1470430787485; Fri, 05 Aug 2016 13:59:47 -0700 (PDT) Subject: Re: [PATCH 1/2] libio: Multiple fixes for open_{w}memstram (BZ#18241 and BZ#20181) To: Andreas Schwab References: <1470418850-22175-1-git-send-email-adhemerval.zanella@linaro.org> <87mvkrj7rn.fsf@linux-m68k.org> Cc: libc-alpha@sourceware.org From: Adhemerval Zanella Message-ID: <7a5d7767-0190-3c17-c17d-37412b8da5b7@linaro.org> Date: Fri, 05 Aug 2016 21:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <87mvkrj7rn.fsf@linux-m68k.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-SW-Source: 2016-08/txt/msg00248.txt.bz2 On 05/08/2016 17:11, Andreas Schwab wrote: > On Fr, Aug 05 2016, Adhemerval Zanella 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 >> + . */ >> + >> +#include >> +#include >> +#include >> + >> + >> +#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. >