🚀 go-pugleaf

RetroBBS NetNews Server

Inspired by RockSolid Light RIP Retro Guy

Article View: demon.ip.developers
Article #374

Re: [another?] memory leak in ka9q/smtpserv

#374
From: Graham E Kinns
Date: Tue, 05 Aug 1997 00:00
81 lines
4422 bytes
In article <870540359snz@pdlmail.demon.co.uk>
           pete@pdlmail.demon.co.uk "Pete" writes:

>Just poking around the source (as one does...) I came across
>a problem in the 'remlist()' func in smtpserv.c - that is,
>the official one on ftp.demon.  Knowing how tight ka9q is
>for mem, it strikes me it would be a good idea to fix it.

Yup, but the only part of the alias expansion that uses it is itself
broken for multiple RCPT TOs - much better to remove the function
entirely.  (IIRC, the way I read it, expansion of the default alias
doesn't match the description in DISPC.TXT either).

>And talking of lists, some memory might be reclaimed by
>rationalising them across ka9q:  I've only looked so far at
>smtpserv and nntpcli with a view to producing a unified
>kill/keep facility and there is duplication of effort even
>in the live code :(  smtpserv.c has a 'struct list ..' and
>nntpcli.c has a (similar) 'struct textline ..' - both with
>their own sets of functions to manipulate them. 

Bear in mind that SMTP and NNTP can be configured in separately.

>And another thing: 'struct textline' is defined in nntpcli.c
>as:
>
>struct textline
>    {
>    struct textline *nextline;      /* pointer to next text line */
>    char text[NNTPMAXLEN];   /* array holding text (shrunk to fit) */
>    };
>
>which is a profligate waste of mem - most kill entries will
>be less than 512 bytes long (one hopes..) so it should be more
>efficient to define 'text' as char * and replace the 'strcpy'
>with a 'strdup' even with the extra 'free' call afterwards.

Ah, but it never actually does a malloc (sizeof (struct textline)) -
the comment is misleading as it's not shrunk but never fully allocated
in the first place - see the MINTEXTLINESIZE #def (using offsetof()
from stddef.h instead would be more portable and protect against
alignment problems).  It saves 4 bytes per list entry and a level of
indirection so it is slightly more efficient than the standard linked
list struct.

The NNTP batch buffers do waste memory by allocating 512 bytes for the
message-id (the average was around 40 last time I checked) - the dbz
library used by the reference implementations only handles keys up to
255 bytes by default so message-ids longer than 254 characters may not
propagate too well.  (I don't know if this restriction is still in
place on the major news peering sites at the moment).  I've reduced
the buffer to 256 and any longer ids are dumped to the log file.

>And another thing!  The 'createkilllists()' func takes no
>special action for empty or comment lines - they get their
>own 500+ byte buffers just like proper entries.  At least the
>smtp code checks for and skips these lines with its lists.
>All such lines now removed from my killfile :-)

The header matching is also pretty dire since each header line is
lower-cased and whitespace-reduced before attempting a match - at the
expense of 200 bytes of memory, I've expanded the kill and keep lists
to 26 lists each, indexed on the first letter of the header (a-z).
Checking only the first character of each incoming header and a much
shorter list to check on a hit is hopefully an performance improvement
for larger kill files.  (No real figures to show it does but I have
broken the 3k/s mark at 14.4k on the TNT lines - this may have
something to do with fixing the byte counts to account for \r\n -> \n
translation).

The downside is not allowing wildcards to match the first character of
a header (which IMHO would be grossly inefficient in the present
implementation) but using * to kill all bodies by default is still
supported - it sets an initial state variable for each header so that
a scan of the kill lists isn't needed, only the keep ones.

Cheers, Graham.
-- 
Graham E. Kinns
Graham@grempc.demon.co.uk
Technical Details
Message-ID: <870774566snz@grempc.demon.co.uk>
Path: arc1-ca.pugleaf.net!unknown!not-for-mail
References: <870540359snz@pdlmail.demon.co.uk>

Show Full Headers
Complete Article Headers
X-Google-Language: ENGLISH,ASCII-7-bit
X-Google-Thread: 10bb05,f2481a491189cc66
X-Google-Attributes: gid10bb05,public
From: newsmaster@grempc.demon.co.uk (Graham E Kinns)
Subject: Re: [another?] memory leak in ka9q/smtpserv
Date: 1997/08/05
Message-ID: <870774566snz@grempc.demon.co.uk>
X-Deja-AN: 262565702
References: <870540359snz@pdlmail.demon.co.uk>
X-Mail2News-User: newsmaster@grempc.demon.co.uk
X-Mail2News-Path: grempc.demon.co.uk
Organization: Home, N Yorks, UK
Reply-To: Graham@grempc.demon.co.uk
Newsgroups: demon.ip.developers