Re: [patch, fortran] PR22539 implement FSEEK intrinsic

[Brooks Moses <brooks dot moses at codesourcery dot com>]

Daniel Franke wrote:
> gcc/fortran:
> 2007-05-01  Daniel Franke  <franke.daniel@gmail.com>
>
> 	PR fortran/22539
>         * intrinsic.c (add_subroutines): Added FSEEK.
>         * intrinsic.h (gfc_resolve_fseek_sub, gfc_check_fseek_sub): New.
>         * iresolve.c (gfc_resolve_fseek_sub): New.
>         * check.c (gfc_check_fseek_sub): New.
>         * intrinsic.texi (FSEEK): Updated.
>
> libgfortran:
> 2007-05-01  Daniel Franke  <franke.daniel@gmail.com>
>
> 	PR fortran/22539
>         * io/intrinsics.c (fseek_sub): New.
>         * gfortran.map (fseek_sub): New.
>
> gcc/testsuite:
> 2007-05-01  Daniel Franke  <franke.daniel@gmail.com>
>
> 	PR fortran/22539
> 	* gfortran.dg/fseek.f90: New test.
>
>
> Regtested on i686-pc-linux-gnu, no regressions.
> Ok for mainline?

Looks good, except for minor documentation edits, and I think the 
library function isn't quite complete.

> Index: intrinsic.texi
> ===================================================================
> --- intrinsic.texi	(revision 124320)
> +++ intrinsic.texi	(working copy)
> @@ -3966,11 +3966,31 @@
>  @cindex file operation, seek
>  @cindex file operation, position
>  
> -Not yet implemented in GNU Fortran.
> -
>  @table @asis
>  @item @emph{Description}:
> +Attempts to move @var{UNIT} to the specified @var{OFFSET}. If @var{WHENCE} 
> +is set to 0, the @var{OFFSET} is taken as an absolute value @code{SEEK_SET},
> +if set to 1, @var{OFFSET} is taken to be relative to the current position 
> +@code{SEEK_CUR}, and if set to 2 relative to the end of the file @code{SEEK_END}.
> +On error, @var{STATUS} is set to a value unequal zero.

I think that should just be "Moves @var{UNIT} to...." -- in general, the 
descriptions of intrinsics indicates what they do when there is no error.

At the end, perhaps we should also say that @var{STATUS} is set to zero 
when there is no error.  And "non-zero value" is a better phrasing than 
"value unequal zero".

If @var{STATUS} is not present, do we just silently fail, or do we emit 
a runtime error?  Either is okay (though I think the runtime error is 
preferable), but it should be documented.

> -@item @emph{Return value}:
> +@multitable @columnfractions .15 .70
> +@item @var{UNIT}   @tab Shall be a scalar of type @code{INTEGER}.
> +@item @var{OFFSET} @tab Shall be a scalar of type @code{INTEGER}.
> +@item @var{WHENCE} @tab Shall be a scalar of type @code{INTEGER}.
> +Its value shall be either 0, 1 or 2.
> +@item @var{STATUS} @tab (Optional) shall be the be a scalar of 
> +type @code{INTEGER}.

There's an erroneous "the be" in that last entry.

> Index: io/intrinsics.c
> ===================================================================
> --- io/intrinsics.c	(revision 124260)
> +++ io/intrinsics.c	(working copy)
> @@ -228,7 +228,34 @@
>      }
>  }
>  
> +/* FSEEK intrinsic */
>  
> +extern void fseek_sub (int *, GFC_IO_INT *, int *, int *);
> +export_proto(fseek_sub);
> +
> +void
> +fseek_sub (int * unit, GFC_IO_INT * offset, int * whence, int * status)
> +{
> +  gfc_unit * u = find_unit (*unit);
> +
> +  if (status) *status = 0;
> +
> +  if (u == NULL || !is_seekable(u->s))
> +    {
> +      if (status) *status = -1; /* EBADF */
> +    }
> +  else if (*whence == 0)
> +    sseek(u->s, *offset);                       /* SEEK_SET */
> +  else if (*whence == 1)
> +    sseek(u->s, file_position(u->s) + *offset); /* SEEK_CUR */
> +  else if (*whence == 2)
> +    sseek(u->s, file_length(u->s) + *offset);   /* SEEK_END */
> +  else if (status)
> +    *status = -1;  /* EINVAL */
> +}

I don't see how this returns a nonzero status if *offset is outside the 
limits of the file such that the sseek fails.  How does that work?

Also, what happens if sseek fails and STATUS isn't supplied; is this a 
runtime error, or a silent failure?

The testcase should check to confirm that FSEEK returns a nonzero value 
when asked to seek outside the limits of the file, and should also (if 
possible; I don't know how we handle runtime errors) check to confirm 
that the correct behavior happens when STATUS isn't supplied.

- Brooks