Thread: [fetchmail-devel] Re: r3995 - trunk

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

On Wed, Nov 10, 2004 at 01:04:34PM -0600, sv...@de... wrote:
> Added:
>    trunk/trio/
> Log:
> Import Trio 1.10 into fetchmail's trunk.

What is this and why do we need it?

-- 
gram

Graham Wilson <gr...@mk...> writes:

> On Wed, Nov 10, 2004 at 01:04:34PM -0600, sv...@de... wrote:
>> Added:
>>    trunk/trio/
>> Log:
>> Import Trio 1.10 into fetchmail's trunk.
>
> What is this and why do we need it?

This is the best GPL-compatible snprintf/sscanf replacement (including
varargs variants) that I've been able to find as a separate project.

The idea is to get rid of insecure and ugly coding practices such as
using [v]sprintf or strcat on older systems that lack [v]snprintf, and
the way we'd seen this integrated in the code was messy:

#ifdef HAVE_SNPRINTF
  snprintf(buf, sizeof buf,     /* safer */
#else
  sprintf(buf,                  /* unsafe */
#endif
     rest, of, arguments);

The diffstat looks like this:

 Makefile.am    |   18 ++++++++++++--
 configure.ac   |   52 ++++++++++++++++++++++++++++++++++++++++
 cram.c         |    4 ---
 driver.c       |   30 +++--------------------
 fetchmail.h    |    5 +++
 imap.c         |    6 ----
 interface.c    |    7 -----
 ipv6-connect.c |   23 +++--------------
 report.c       |   46 -----------------------------------
 sink.c         |   59 +++++-----------------------------------------
 smtp.c         |   34 ++++----------------------
 socket.c       |   15 -----------
 transact.c     |   73 ++++-----------------------------------------------------
 13 files changed, 103 insertions(+), 269 deletions(-)

Not counting the first two files, we are now down by over 200 lines of
half-baked C code.

I'm not fixed on Trio, if you know something better, we can flush Trio
and use something else, but I'm not willing to allow the ugly old
structure back in.

-- 
Matthias Andree

On Wed, Nov 10, 2004 at 09:14:37PM +0100, Matthias Andree wrote:
> Graham Wilson <gr...@mk...> writes:
> > What is this and why do we need it?
> 
> This is the best GPL-compatible snprintf/sscanf replacement (including
> varargs variants) that I've been able to find as a separate project.

Why do we want it as a separate project?

> The idea is to get rid of insecure and ugly coding practices such as
> using [v]sprintf or strcat on older systems that lack [v]snprintf, and
> the way we'd seen this integrated in the code was messy:
> 
> [...]

I agree that it is a good idea to get rid of sprintf throughout the
code. Thanks for spearheading the effort.

> I'm not fixed on Trio, if you know something better, we can flush Trio
> and use something else,

I would have gone with a single file implementation of snprintf instead
of using the whole Trio. I don't see why we need to add 54 files when we
could just add one. For example, mutt is known to have a good snprintf
implmentation.

What would you think about using that?

> but I'm not willing to allow the ugly old structure back in.

Agreed.

-- 
gram

Graham Wilson <gr...@mk...> writes:

> On Wed, Nov 10, 2004 at 09:14:37PM +0100, Matthias Andree wrote:
>> Graham Wilson <gr...@mk...> writes:
>> > What is this and why do we need it?
>> 
>> This is the best GPL-compatible snprintf/sscanf replacement (including
>> varargs variants) that I've been able to find as a separate project.
>
> Why do we want it as a separate project?

Ripping code out of other software is harder to track WRT updates.

>> I'm not fixed on Trio, if you know something better, we can flush Trio
>> and use something else,
>
> I would have gone with a single file implementation of snprintf instead
> of using the whole Trio. I don't see why we need to add 54 files when we
> could just add one. For example, mutt is known to have a good snprintf
> implmentation.
>
> What would you think about using that?

What is the claim mutt had a "good snprintf" implementation based on?

It's one of the hordes of the Patrick Powell snprintf.c derivates, and
as such, one that's both incomplete and pretty broken - it uses
va_arg(args, short int) which is outright nonsense given the promotion
rules, and it doesn't get return values right as documented.

Note that the tarball only includes a small subset of the Trio files,
namely these (try "make distdir" to figure):

total 328K
 20K CHANGES       160K trio.c      24K trionan.c   44K triostr.c
4.0K README        8.0K trio.h     4.0K trionan.h   12K triostr.h
 36K regression.c  8.0K triodef.h  8.0K triop.h

I'll have a look at Gary V. Vaughan's snprintfv but the tarball also is
463k in size so I'm not sure if it'll be much lighter than Trio.

http://savannah.nongnu.org/projects/libsnprintfv
ftp://alpha.gnu.org/gnu/snprintfv

-- 
Matthias Andree

On Fri, Nov 12, 2004 at 11:54:51PM +0100, Matthias Andree wrote:
> Graham Wilson <gr...@mk...> writes:
> > Why do we want it as a separate project?
> 
> Ripping code out of other software is harder to track WRT updates.

I'm not really concerned as much about updates to an snprintf
implementation; I wouldn't think there would need to be much updating.
I'm more concerned with adding a whole other project into the fetchmail
tarball.

> > I would have gone with a single file implementation of snprintf instead
> > of using the whole Trio. I don't see why we need to add 54 files when we
> > could just add one. For example, mutt is known to have a good snprintf
> > implmentation.
> 
> What is the claim mutt had a "good snprintf" implementation based on?

Probably some unfounded claim that I read somewhere else I suppose. If
you don't think the implementation is good, I'm fine with going with
something else.

> It's one of the hordes of the Patrick Powell snprintf.c derivates, and
> as such, one that's both incomplete and pretty broken - it uses
> va_arg(args, short int) which is outright nonsense given the promotion
> rules, and it doesn't get return values right as documented.

Well, I thought that was the case. I'm fine not using though if you
don't think the code is good.

What about the implementation at [1]? The downside is that it doesn't
support a number of conversion characters (f, e, E, g, G, lc, ls, n),
none of which we seem to use though. There are a few other small things
missing which I don't think we use (or will) either.

On the other hand, the source doesn't seem to be the derived from
Powell's implementation and the return semantics seem correct. The code
is also distributed in tarball form, so it seems updates would be easy
to track (though there hasn't been a release since late 2000 it seems).

-- 
gram

On Thu, Nov 18, 2004 at 12:41:12AM -0600, Graham Wilson wrote:
> What about the implementation at [1]? The downside is that it doesn't
> support a number of conversion characters (f, e, E, g, G, lc, ls, n),
> none of which we seem to use though. There are a few other small things
> missing which I don't think we use (or will) either.
> 
> On the other hand, the source doesn't seem to be the derived from
> Powell's implementation and the return semantics seem correct. The code
> is also distributed in tarball form, so it seems updates would be easy
> to track (though there hasn't been a release since late 2000 it seems).

[1] http://www.ijs.si/software/snprintf/

-- 
gram

On Thu, Nov 18, 2004 at 12:41:12AM -0600, Graham Wilson wrote:
> On Fri, Nov 12, 2004 at 11:54:51PM +0100, Matthias Andree wrote:
> > Graham Wilson <gr...@mk...> writes:
> > > Why do we want it as a separate project?
> > 
> > Ripping code out of other software is harder to track WRT updates.
> 
> I'm not really concerned as much about updates to an snprintf
> implementation; I wouldn't think there would need to be much updating.
> I'm more concerned with adding a whole other project into the fetchmail
> tarball.

What do other projects do with regard to snprintf or lack of it? How many
Unix systems lack snprintf these days?

According to the FreeBSD manpage,

     the snprintf() and vsnprintf()
     functions conform to ISO/IEC 9899:1999 (`ISO C99'').

Would it be reasonable simply to expect a decent O/S to have these functions
now? I don't know if SunOS 4.1 has snprintf or not, but if it doesn't and
you're still running it, you'll have worse problems than not being able to
compile fetchmail.

If there is a free-standing external snprintf library, then we could just
point people at it and tell them to install it first if it's needed.

Regards,

Brian.

Graham Wilson <gr...@mk...> writes:

> Matthias Andree:

>> Ripping code out of other software is harder to track WRT updates.
>
> I'm not really concerned as much about updates to an snprintf
> implementation;

I am.

> I wouldn't think there would need to be much updating.

More than 4/5 of the snprintf replacements I've looked at were broken in
one way or another.

> I'm more concerned with adding a whole other project into the fetchmail
> tarball.

We aren't doing that.

>> What is the claim mutt had a "good snprintf" implementation based on?
>
> Probably some unfounded claim that I read somewhere else I suppose. If
> you don't think the implementation is good, I'm fine with going with
> something else.

I think the mutt implementation is too incomplete to be useful for
fetchmail.

> What about the implementation at [1]? The downside is that it doesn't
> support a number of conversion characters (f, e, E, g, G, lc, ls, n),
> none of which we seem to use though. There are a few other small things
> missing which I don't think we use (or will) either.

(from different mail)
> [1] http://www.ijs.si/software/snprintf/

It doesn't support %n$ either, which is needed for internationalized
projects, so it, too, is out of the game. I haven't yet had the time to
look _closely_ at snprintfv.

Brian Candler <B.C...@po...> writes:

> What do other projects do with regard to snprintf or lack of it? How many
> Unix systems lack snprintf these days?

I don't know, but I wouldn't want to break compatibility in a patchlevel
or minot update. If we drop all the cruft,  we need to call it 7.0 - and
we'd instead need to get out 6.2.6 or 6.3.0 soon.  We can change
requirements an drop compatibility cruft for a later major version.

> According to the FreeBSD manpage,
>
>      the snprintf() and vsnprintf()
>      functions conform to ISO/IEC 9899:1999 (`ISO C99'').
>
> Would it be reasonable simply to expect a decent O/S to have these functions
> now? I don't know if SunOS 4.1 has snprintf or not,

It doesn't, but SunOS older than 5.7 (IIRC) isn't supported by the
vendor any more.

> If there is a free-standing external snprintf library, then we could just
> point people at it and tell them to install it first if it's needed.

The replacements I've looked at all assumed to be packaged as integral
part of another application.

-- 
Matthias Andree

On Thu, Nov 18, 2004 at 12:35:16PM +0100, Matthias Andree wrote:
> Brian Candler <B.C...@po...> writes:
> > What do other projects do with regard to snprintf or lack of it? How many
> > Unix systems lack snprintf these days?
> 
> I don't know, but I wouldn't want to break compatibility in a patchlevel
> or minot update. If we drop all the cruft,  we need to call it 7.0 - and
> we'd instead need to get out 6.2.6 or 6.3.0 soon.  We can change
> requirements an drop compatibility cruft for a later major version.

I think this is probably a good path to go down. Forget about sprintf
usage for now, and consider our alternatives after we have a few more
minor 6.2.x releases done.

At that point we could consider dropping support for systems without
snprintf, or figuring what other code to integrate.

-- 
gram

Thread: [fetchmail-devel] Re: r3995 - trunk

Client daemon to move mail from POP and IMAP to your local computer

fetchmail-devel