Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz <drow at false dot org>]

On Fri, Sep 09, 2005 at 11:23:14AM -0600, Shaun Jackman wrote:
> 2005/9/8, Richard Earnshaw <rearnsha@gcc.gnu.org>:
> > Two problems I can immediately see with this.
> > 
> > 1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
> > can't rely on it being defined.  I think for this case you can probably
> > just define it to 1024 anyway if it's missing, but see
> > libiberty/lrealpath.c if you want the gory details.
> > 
> > 2) If you do overflow the path limit, you need to set the simulator's
> > errno value and return.  Use cb_host_to_target_errno(sim_callback,
> > ENAMETOOLONG) to set it.
> > 
> > R.
> 
> I've changed PATH_MAX from POSIX to FILENAME_MAX from ANSI and checked
> if the path limit overflowed.

I'm sorry we even got into this... but no, you can not use FILENAME_MAX
this way:

Macro: int FILENAME_MAX
    The value of this macro is an integer constant expression that
represents the maximum length of a file name string. It is defined in
`stdio.h'.

    Unlike PATH_MAX, this macro is defined even if there is no actual
limit imposed. In such a case, its value is typically a very large
number. This is always the case on the GNU system.

    Usage Note: Don't use FILENAME_MAX as the size of an array in which
to store a file name! You can't possibly make an array that big! Use
dynamic allocation (see section 3.2 Allocating Storage For Program
Data) instead. 


That's from the glibc manual, but it's generally good advice.  I don't
know if Hurd does this at the moment, looking at current glibc, but it
certainly did at one point.  Also, some older systems defined
FILENAME_MAX as 14.

I'd recommend following Richard's suggestion: use PATH_MAX, if it's not
there, default to 1K.

> -  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
> -    ;
> +  for (i = 0; i < sizeof buf; i++)
> +    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
> +      break;
> +  if (i == sizeof buf) {
> +    state->Reg[0] = -1;
> +    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
> +    return;
> +  }

You might want a helper function for this, since it happens in a lot of
places.  Also, braces on their own lines.

Otherwise it looks fine to me.


-- 
Daniel Jacobowitz
CodeSourcery, LLC