headers
Paul Eggert | 1 Jun 2006 01:15
Favicon

Re: strfile: new module

Simon Josefsson <jas <at> extundo.com> writes:

> I think we should be able to assume free (NULL) works though

Yes.  To my knowledge the last major system for which the 'free'
module was needed was SunOS 4.1.4 aka Solaris 1.1.2.  Sun stopped
patching SunOS 4.1.4 in 2000 and stopped supporting it in 2003 (see
<http://www.sun.com/service/eosl/solaris/solaris_vintage_eol_5.2005.html>)
so we don't need to worry about it any more.
Permalink | Reply | 4 comments |
headers
Simon Josefsson | 1 Jun 2006 09:59

Re: strfile: new module

Paul Eggert <eggert <at> CS.UCLA.EDU> writes:

> Simon Josefsson <jas <at> extundo.com> writes:
>
>> I think we should be able to assume free (NULL) works though
>
> Yes.  To my knowledge the last major system for which the 'free'
> module was needed was SunOS 4.1.4 aka Solaris 1.1.2.  Sun stopped
> patching SunOS 4.1.4 in 2000 and stopped supporting it in 2003 (see
> <http://www.sun.com/service/eosl/solaris/solaris_vintage_eol_5.2005.html>)
> so we don't need to worry about it any more.

Fixed.

What about realloc (NULL, ...)?  Should read-file depend on the
realloc module if it use that?

/Simon
Permalink | Reply | 3 comments |
headers
Simon Josefsson | 1 Jun 2006 10:03

realloc buggy?

For read-file, I looked at realloc, and can't help but feel it is
buggy.  Here is the function:

void *
rpl_realloc (void *p, size_t n)
{
  if (n == 0)
    {
      n = 1;

      /* In theory realloc might fail, so don't rely on it to free.  */
      free (p);
      p = NULL;
    }

  if (p == NULL)
    return malloc (n);
  return realloc (p, n);
}

If, for a non-NULL p, realloc (p, 0) returns NULL, i.e., if the malloc
fails, then p has been freed, which doesn't seem right?  If realloc
returns NULL, the pointer should not have been freed, according to my
realloc(3):

  If realloc()
       fails the original block is left untouched; it is not freed or moved.

What about this instead?
(Continue reading)

Permalink | Reply | 3 comments |
headers
Paul Eggert | 1 Jun 2006 10:20
Favicon

Re: strfile: new module

Simon Josefsson <jas <at> extundo.com> writes:

> What about realloc (NULL, ...)?

In practice that is as portable as free (NULL).  They were
standardized at the same time.
Permalink | Reply | 2 comments |
headers
Ralf Wildenhues | 1 Jun 2006 10:29
Picon
Picon

Re: strfile: new module

* Paul Eggert wrote on Thu, Jun 01, 2006 at 10:20:58AM CEST:
> Simon Josefsson <jas <at> extundo.com> writes:
> 
> > What about realloc (NULL, ...)?
> 
> In practice that is as portable as free (NULL).  They were
> standardized at the same time.

Well, with the difference being here that realloc, similar to malloc,
may not do what you expect from its glibc implementations, i.e., return
non-NULL upon a request of zero size (which is the main reason for
the Gnulib realloc module).

Cheers,
Ralf
Permalink | Reply | 1 comment |
headers
Paul Eggert | 1 Jun 2006 10:33
Favicon

Re: realloc buggy?

Simon Josefsson <jas <at> extundo.com> writes:

> What about this instead?

Unfortunately that is buggy too, if the goal is to emulate glibc.
With glibc, if p is not null, realloc (p, 0) always returns NULL.
Conversely, realloc (NULL, 0) is equivalent to malloc (0) and
typically returns non-NULL.  It is kind of confusing.
Permalink | Reply | 2 comments |
headers
Simon Josefsson | 1 Jun 2006 12:47

Re: strfile: new module

Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:

> * Paul Eggert wrote on Thu, Jun 01, 2006 at 10:20:58AM CEST:
>> Simon Josefsson <jas <at> extundo.com> writes:
>> 
>> > What about realloc (NULL, ...)?
>> 
>> In practice that is as portable as free (NULL).  They were
>> standardized at the same time.
>
> Well, with the difference being here that realloc, similar to malloc,
> may not do what you expect from its glibc implementations, i.e., return
> non-NULL upon a request of zero size (which is the main reason for
> the Gnulib realloc module).

Read-file doesn't call realloc with size=0 (but it does call it with
pointer=NULL), so I've dropped the dependency on the realloc module in
read-file.

/Simon
Permalink | Reply | 0 comments |
headers
Simon Josefsson | 1 Jun 2006 12:51

Re: realloc buggy?

Paul Eggert <eggert <at> CS.UCLA.EDU> writes:

> Simon Josefsson <jas <at> extundo.com> writes:
>
>> What about this instead?
>
> Unfortunately that is buggy too, if the goal is to emulate glibc.
> With glibc, if p is not null, realloc (p, 0) always returns NULL.

And free's p, presumably?

> Conversely, realloc (NULL, 0) is equivalent to malloc (0) and
> typically returns non-NULL.  It is kind of confusing.

Yes, quite confusing.  It's a bad that the libc manual doesn't discuss
the semantics for NEWSIZE==0, so the behaviour you describe is not
documented as far as I can tell.

The glibc behaviour here seems like something we shouldn't encourage
IMHO.

/Simon
Permalink | Reply | 1 comment |
headers
Bruno Haible | 1 Jun 2006 14:00

Re: strfile: new module

Simon Josefsson wrote:
> Here's the module.  Only lighted tested, mostly to get feedback on the
> approach, but if you agree with it, detailed review would be useful.

Looks already quite good. Only minor nits:

> +/* Read a STREAM and return a newly allocated string with the content,
> +   and set LENGTH to the length of the string.

It sets *LENGTH, not LENGTH, I think.

>                                                  The string is
> +   zero-terminated, but the terminating zero character is not counted

I'd prefer to write "zero byte" instead of "zero character", because a
character nowadays can generally be multibyte.

> +   in the LENGTH variable.  On errors, return NULL, sets errno and

On systems like mingw (which don't attempt POSIX compliance) errno is
not set when fread() fails.

> +   LENGTH is undefined. */

*LENGTH here as well.

> +	  char *tmp;

I would rename this variable to 'new_buf'.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Simon Josefsson | 1 Jun 2006 16:37

Re: strfile: new module

Bruno Haible <bruno <at> clisp.org> writes:

> Simon Josefsson wrote:
>> Here's the module.  Only lighted tested, mostly to get feedback on the
>> approach, but if you agree with it, detailed review would be useful.
>
> Looks already quite good. Only minor nits:
>
>> +/* Read a STREAM and return a newly allocated string with the content,
>> +   and set LENGTH to the length of the string.
>
> It sets *LENGTH, not LENGTH, I think.

Fixed.

>>                                                  The string is
>> +   zero-terminated, but the terminating zero character is not counted
>
> I'd prefer to write "zero byte" instead of "zero character", because a
> character nowadays can generally be multibyte.

Changed.

>> +   in the LENGTH variable.  On errors, return NULL, sets errno and
>
> On systems like mingw (which don't attempt POSIX compliance) errno is
> not set when fread() fails.

I've changed it to just say that it doesn't distort errno:
(Continue reading)

Permalink | Reply | 0 comments |

Gmane