This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, libgfortran, 4.5] PR25561, 37754: New low level I/O library
- From: Janne Blomqvist <blomqvist dot janne at gmail dot com>
- To: Thomas Schwinge <thomas at codesourcery dot com>
- Cc: gfortran <fortran at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 10 Sep 2014 00:25:47 +0300
- Subject: Re: [Patch, libgfortran, 4.5] PR25561, 37754: New low level I/O library
- Authentication-results: sourceware.org; auth=none
- References: <4962853A dot 3080608 at gmail dot com> <87egvq2hfj dot fsf at kepler dot schwinge dot homeip dot net>
On Fri, Sep 5, 2014 at 6:52 PM, Thomas Schwinge <thomas@codesourcery.com> wrote:
> Hi!
>
> On Tue, 06 Jan 2009 00:10:02 +0200, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> attached is a substantially reworked low level I/O library for gfortran.
>> [...]
>
> Due to a recent improvement in GCC, it is now pointing out an issue in
> the following code:
>
>> 2009-01-05 Janne Blomqvist <jb@gcc.gnu.org>
>>
>> PR libfortran/25561 libfortran/37754
>> * [...]
>
>> --- a/libgfortran/io/transfer.c
>> +++ b/libgfortran/io/transfer.c
>> [...]
>> if ((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0)
>> - dtp->u.p.size_used += (GFC_IO_INT) nread;
>> + dtp->u.p.size_used += (GFC_IO_INT) *nbytes;
>>
>> - if (nread != *nbytes)
>> - { /* Short read, this shouldn't happen. */
>> - if (likely (dtp->u.p.current_unit->pad_status == PAD_YES))
>> - *nbytes = nread;
>> - else
>> + if (norig != *nbytes)
>> + {
>> + /* Short read, this shouldn't happen. */
>> + if (!dtp->u.p.current_unit->pad_status == PAD_YES)
>> {
>> generate_error (&dtp->common, LIBERROR_EOR, NULL);
>> source = NULL;
>> }
>> }
>>
>> - dtp->u.p.current_unit->strm_pos += (gfc_offset) nread;
>> + dtp->u.p.current_unit->strm_pos += (gfc_offset) *nbytes;
>> [...]
>
> ../../../source/libgfortran/io/transfer.c: In function 'read_block_form':
> ../../../source/libgfortran/io/transfer.c:478:46: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
> if (!dtp->u.p.current_unit->pad_status == PAD_YES)
> ^
>
> I assume the following is what is intended, but I have not made an
> attempt to verify this:
>
> diff --git libgfortran/io/transfer.c libgfortran/io/transfer.c
> index af2932c..710de7a 100644
> --- libgfortran/io/transfer.c
> +++ libgfortran/io/transfer.c
> @@ -475,7 +475,7 @@ read_block_form (st_parameter_dt *dtp, int * nbytes)
> if (norig != *nbytes)
> {
> /* Short read, this shouldn't happen. */
> - if (!dtp->u.p.current_unit->pad_status == PAD_YES)
> + if (!(dtp->u.p.current_unit->pad_status == PAD_YES))
> {
> generate_error (&dtp->common, LIBERROR_EOR, NULL);
> source = NULL;
>
>
> GrÃÃe,
> Thomas
Hi,
thanks for finding the issue. Looking at the code, I fixed it with the
slightly different patch below. Committed to trunk as obvious after
regtesting on x86_64-unknown-linux-gnu.
Index: ChangeLog
===================================================================
--- ChangeLog (revision 215091)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-09-10 Janne Blomqvist <jb@gcc.gnu.org>
+
+ * io/transfer.c (read_block_form): Fix pad status check (found by
+ Thomas Schwinge with -Wlogical-not-parentheses).
+
2014-08-31 Tobias Burnus <burnus@net-b.de>
* caf/libcaf.h (_gfortran_caf_send, _gfortran_caf_get,
Index: io/transfer.c
===================================================================
--- io/transfer.c (revision 215091)
+++ io/transfer.c (working copy)
@@ -475,7 +475,7 @@ read_block_form (st_parameter_dt *dtp, i
if (norig != *nbytes)
{
/* Short read, this shouldn't happen. */
- if (!dtp->u.p.current_unit->pad_status == PAD_YES)
+ if (dtp->u.p.current_unit->pad_status == PAD_NO)
{
generate_error (&dtp->common, LIBERROR_EOR, NULL);
source = NULL;
--
Janne Blomqvist