[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: strfile: new module
From: |
Simon Josefsson |
Subject: |
Re: strfile: new module |
Date: |
Wed, 31 May 2006 22:41:03 +0200 |
User-agent: |
Gnus/5.110006 (No Gnus v0.6) Emacs/22.0.50 (gnu/linux) |
Paul Eggert <address@hidden> writes:
> Looks nice. A couple of comments:
>
>> + count = fread (buf + size, 1, BUFSIZ, stream);
>
> BUFSIZ is too small on many systems, for backward compatibility reasons.
> Why not change that "BUFSIZ" to "alloc - size - 1"?
Done.
> Also, for the common case of regular files that you have opened,
> you can use fstat to cheaply find out an estimated size for the file,
> and use that estimate rather than guessing and growing the buffer.
> The estimate might be wrong (due to binary file translation, or
> growing sizes), but still, it's better than a guess.
>
> Even if it is a regular file that you have not opened, you can still
> estimate the size of the remaining part of the file by subtracting
> lseek (..., SEEK_CUR) from fstat.
Yup. It's slightly less portable, perhaps, but if someone writes the
code (especially the M4 test), adding this seems like a good idea.
It will probably be easier to write the code once the current module
has been installed though. The mmap approach is even less portable,
and also require a call to munmap, so I'm less sure about it. I'm
just looking for something very simple to read X.509 certificates,
Kerberos tickets etc, which are typically only a few kb's large.
I'll start to use the module in GnuTLS and Shishi to see how it works
now.
>> + if (buf)
>> + free (buf);
>
> How about just 'free (buf);'; that's smaller.
I prefer that to, but Ralf said to depend on the "free" module to make
sure free (NULL) works. The free module is GPL, and I need this
module to be LGPL, so I can't use it, hence handling this in the code.
I think we should be able to assume free (NULL) works though, even my
K&R says it should work, and without depending on the free module.
/Simon
Re: [bug-gnulib] strfile: new module, Bruno Haible, 2006/05/30
Re: strfile: new module, Simon Josefsson, 2006/05/31
Re: strfile: new module, Jim Meyering, 2006/05/31