Re: [patch, libfortran] PR 23815: Unformatted data conversion via environment variablen

[Steve Kargl <sgk at troutmask dot apl dot washington dot edu>]

On Sun, Jan 22, 2006 at 05:26:08PM +0100, Thomas Koenig wrote:

I'm still bootstrapping your patch for testing.  I think it's
technically sound, but there are several formatting problems.
I noted these below.

I also forgot to send you an email about an IRC exchange I had
with Janis Johnson.  At this time, setting environmental variables
in dejagnu isn't possible.  The example I gave Janis was something
like

! { dg-do compile }
! { dg-setenv "VARIABLE_NAME" "VALUE" }

> Index: gcc/fortran/lang.opt
> ===================================================================
> --- gcc/fortran/lang.opt	(revision 109982)
> +++ gcc/fortran/lang.opt	(working copy)
> @@ -201,4 +201,20 @@ fshort-enums
>  Fortran
>  Use the narrowest integer type possible for enumeration types
>  
> +fconvert=little-endian
> +Fortran

Fortran RejectNegative

We don't need/want -fno-little-endian.  Add RejectNegative to others.

> Index: libgfortran/runtime/environ.c
> ===================================================================
> --- libgfortran/runtime/environ.c	(revision 109982)
> +++ libgfortran/runtime/environ.c	(working copy)
> @@ -61,6 +61,7 @@ typedef struct variable
>  }
>  variable;
>  
> +static void init_unformatted (variable *);
>  
>  /* print_spaces()-- Print a particular number of spaces */

Comments are terminated with ".  */" That is, period and 2 spaces.

> +  /* GFORTRAN_CONVERT_UNIT */
> +  {"GFORTRAN_CONVERT_UNIT", 0, 0, init_unformatted, show_string,
> +   "Set format for unformatted files", 0},

The comment above is redundant.  You may want to expand the comment 
to describe the nature of each entry.

> +/* Defines for the tokens.  Other valid tokens are ',', ':', '-'. */

Two spaces.

> +#define NATIVE   257
> +#define SWAP     258
> +#define BIG      259
> +#define LITTLE   260
> +/* Some space for additional tokens later */

Period and two spaces.

> +static int unit_num;       /* The last unit number read.  */
> +static int unit_count;     /* The number of units found. */

Space.

> +static unit_convert endian; /* Current endianness */
> +
> +static unit_convert def; /* Default as specified (if any) */

Period and space.

> +static int
> +match_word (const char *word, int tok)
> +{
> +  int res;
> +
> +  if (strncasecmp(p, word, strlen (word)) == 0)

Space "strncasecmp (p..."

> +    {
> +      p += strlen(word);

Space "strlen (word)"

> +match_integer (void)
> +{
> +  unit_num = 0;
> +  while (isdigit(*p))

Space "isdigit ("

> +/* This reads the next token from the GFORTRAN_CONVERT_UNITS variable.
> +   Returned values are END, ILLEGAL,*/

Is this comment complete?  Need two spaces.

> +static void
> +push_token(void)

Space "push_token (void)"

> +    {
> +      for (j=n_elist; j>=i; j--)
> +	elist[j+1] = elist[j];

You spaces around the operators.

for (j = n_elist; j >= i; j--

> +	for (i=unit2; i<=unit1; i++)
> +	  mark_single(i);
> +      else
> +	for (i=unit1; i<=unit2; i++)
> +	  mark_single(i);

Spaces around the operators.

> +static int
> +do_parse(void)

do_parse (void)

> +  tok = next_token();

next_token ();

> +      tok = next_token();

next_token ();

> +	  if (next_token() != ':')

next_token ()

> +	  if (next_token() != ':')

next_token ()

> +	  push_token();

push_token ();

> +	  tok = next_token();

next_token ()

> +	  tok = next_token();

Ditto.

> +	      if (next_token() != INTEGER)

Ditto.

> +	      mark_range(unit1, unit_num);

mark_range (

> +	      tok = next_token();

next_token ()

> +void init_unformatted (variable * v)

void
init_unformatted (...

> +  val = getenv (v->name);

Do we need a "#ifdef HAVE_GETENV" around this?  Likely targets with
a problem are cygwin or MingW.

> +unit_convert get_unformatted_convert (int unit)

unit_convert
get_unformatted_convert (int unit)

-- 
Steve