From patchwork Tue Mar 18 20:31:56 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 150 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (caibbdcaaahc.dreamhost.com [208.113.200.72]) by wilcox.dreamhost.com (Postfix) with ESMTP id B32F436009F for ; Tue, 18 Mar 2014 13:31:21 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14307373) id 64AEF4116FD9A; Tue, 18 Mar 2014 13:31:21 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 3FE994116FD95 for ; Tue, 18 Mar 2014 13:31:21 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type; q=dns; s=default; b=Z1nXLvZ7YcHDS1arD1E3BFAxhzIu/ ixVRPCdKysCom9hGkbx8g9mEdTfffTDzCITfv/QEV5Hkxs23FuxK3KlMEujPhMbY OZ2fSiwUvrWWHwCtY//I6jhS5xxSVPCfnlxwzU6q3d/C3H5OXySeKSAM1oZOcJYb kgRlAtU9HrglCo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type; s=default; bh=pglUOl8jIxZgfh36QZIl51W1ez8=; b=aSa yQi13NtacHJG7/CvpFou3m84RPTWKUK9ftypyCfsaR3JzEGow6E2Sp7SdD7j6/1M UhueilLM8eMLjt2C5bn8sS5BxPADEbS03076ls2dXLakTln7dvwP20WHikAcFTVU clOi0ntm1smnjnn3ZceB27KlXWujOK1cBfGrMryE= Received: (qmail 31168 invoked by alias); 18 Mar 2014 20:31:18 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 31150 invoked by uid 89); 18 Mar 2014 20:31:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Date: Wed, 19 Mar 2014 02:01:56 +0530 From: Siddhesh Poyarekar To: libc-alpha@sourceware.org Subject: [PATCH] Fix offset computation for append+ mode on switching from read (BZ #16724) Message-ID: <20140318203155.GA3762@spoyarek.pnq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.22.1-rc1 (2013-10-16) X-DH-Original-To: glibc@patchwork.siddhesh.in The offset computation in ftell in write mode uses the fact that _IO_read_end is kept in sync with the external file offset. This however is not true when O_APPEND is in effect since switching to write mode ought to send the external file offset to the end of file without making the necessary adjustment to _IO_read_end. Hence in append mode, offset computation when writing should only consider the effect of unflushed writes, i.e. from _IO_write_base to _IO_write_ptr. Tested on x86_66. OK to commit? Siddhesh [BZ #16724] * libio/tst-ftell-append.c: New test case. * libio/Makefile (tests): Add test case. * libio/fileops.c (do_ftell): Don't trust _IO_read_end when in append mode. * libio/wfileops.c (do_ftell_wide): Likewise. --- libio/Makefile | 3 +- libio/fileops.c | 12 +++- libio/tst-ftell-append.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++ libio/wfileops.c | 13 +++- 4 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 libio/tst-ftell-append.c diff --git a/libio/Makefile b/libio/Makefile index 69c25c0..4bedfad 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -60,7 +60,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ tst-wmemstream1 tst-wmemstream2 \ bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ - tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler + tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ + tst-ftell-append ifeq (yes,$(build-shared)) # Add test-fopenloc only if shared library is enabled since it depends on # shared localedata objects. diff --git a/libio/fileops.c b/libio/fileops.c index cf68dbf..204cfea 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -91,7 +91,9 @@ extern struct __gconv_trans_data __libio_translit attribute_hidden; The position in the buffer that corresponds to the position in external file system is normally _IO_read_end, except in putback - mode, when it is _IO_save_end. + mode, when it is _IO_save_end and also when the file is in append mode, + since switching from read to write mode automatically sends the position in + the external file system to the end of file. If the field _fb._offset is >= 0, it gives the offset in the file as a whole corresponding to eGptr(). (?) @@ -966,6 +968,14 @@ do_ftell (_IO_FILE *fp) /* Adjust for unflushed data. */ if (!was_writing) offset -= fp->_IO_read_end - fp->_IO_read_ptr; + /* We don't trust _IO_read_end to represent the current file offset when + writing in append mode because the value would have to be shifted to + the end of the file during a flush. Use the write base instead, along + with the new offset we got above when we did a seek to the end of the + file. */ + else if (append_mode) + offset += fp->_IO_write_ptr - fp->_IO_write_base; + /* For all other modes, _IO_read_end represents the file offset. */ else offset += fp->_IO_write_ptr - fp->_IO_read_end; } diff --git a/libio/tst-ftell-append.c b/libio/tst-ftell-append.c new file mode 100644 index 0000000..57298ac --- /dev/null +++ b/libio/tst-ftell-append.c @@ -0,0 +1,168 @@ +/* Verify that ftell returns the correct value after a read and a write on a + file opened in a+ mode. + Copyright (C) 2014 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 +#include +#include +#include +#include + +/* data points to either char_data or wide_data, depending on whether we're + testing regular file mode or wide mode respectively. Similarly, + fputs_func points to either fputs or fputws. data_len keeps track of the + length of the current data and file_len maintains the current file + length. */ +static void *buf; +static char char_buf[4]; +static wchar_t wide_buf[4]; +static const void *data; +static const char *char_data = "abcdefghijklmnopqrstuvwxyz"; +static const wchar_t *wide_data = L"abcdefghijklmnopqrstuvwxyz"; +static size_t data_len; +static size_t file_len; + +typedef int (*fputs_func_t) (const void *data, FILE *fp); +fputs_func_t fputs_func; + +typedef void *(*fgets_func_t) (void *s, int size, FILE *stream); +fgets_func_t fgets_func; + +static int do_test (void); + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +static FILE * +init_file (const char *filename) +{ + FILE *fp = fopen (filename, "w"); + if (fp == NULL) + { + printf ("fopen: %m\n"); + return NULL; + } + + int written = fputs_func (data, fp); + + if (written == EOF) + { + printf ("fputs failed to write data\n"); + fclose (fp); + return NULL; + } + + file_len = data_len; + + fclose (fp); + + fp = fopen (filename, "a+"); + if (fp == NULL) + { + printf ("fopen(a+): %m\n"); + return NULL; + } + + return fp; +} + +static int +do_one_test (const char *filename) +{ + FILE *fp = init_file (filename); + + if (fp == NULL) + return 1; + + void *ret = fgets_func (buf, sizeof (buf), fp); + + if (ret == NULL) + { + printf ("read failed: %m\n"); + fclose (fp); + return 1; + } + + int written = fputs_func (data, fp); + + if (written == EOF) + { + printf ("fputs failed to write data\n"); + fclose (fp); + return 1; + } + + file_len += data_len; + + long off = ftell (fp); + + if (off != file_len) + { + printf ("Incorrect offset %ld, expected %zu\n", off, file_len); + fclose (fp); + return 1; + } + else + printf ("Correct offset %ld after write.\n", off); + + return 0; +} + +/* Run the tests for regular files and wide mode files. */ +static int +do_test (void) +{ + int ret = 0; + char *filename; + int fd = create_temp_file ("tst-ftell-append-tmp.", &filename); + + if (fd == -1) + { + printf ("create_temp_file: %m\n"); + return 1; + } + + close (fd); + + /* Tests for regular files. */ + puts ("Regular mode:"); + fputs_func = (fputs_func_t) fputs; + fgets_func = (fgets_func_t) fgets; + data = char_data; + buf = char_buf; + data_len = strlen (char_data); + ret |= do_one_test (filename); + + /* Tests for wide files. */ + puts ("Wide mode:"); + if (setlocale (LC_ALL, "en_US.UTF-8") == NULL) + { + printf ("Cannot set en_US.UTF-8 locale.\n"); + return 1; + } + fputs_func = (fputs_func_t) fputws; + fgets_func = (fgets_func_t) fgetws; + data = wide_data; + buf = wide_buf; + data_len = wcslen (wide_data); + ret |= do_one_test (filename); + + return ret; +} diff --git a/libio/wfileops.c b/libio/wfileops.c index 3199861..f123add 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -713,9 +713,16 @@ do_ftell_wide (_IO_FILE *fp) offset += outstop - out; } - /* _IO_read_end coincides with fp._offset, so the actual file - position is fp._offset - (_IO_read_end - new_write_ptr). */ - offset -= fp->_IO_read_end - fp->_IO_write_ptr; + /* We don't trust _IO_read_end to represent the current file offset + when writing in append mode because the value would have to be + shifted to the end of the file during a flush. Use the write base + instead, along with the new offset we got above when we did a seek + to the end of the file. */ + if (append_mode) + offset += fp->_IO_write_ptr - fp->_IO_write_base; + /* For all other modes, _IO_read_end represents the file offset. */ + else + offset += fp->_IO_write_ptr - fp->_IO_read_end; } }