Patch for Distribution header problem

Clifton Royston cliftonr at lava.net
Fri Nov 19 19:16:50 UTC 1999


On Fri, Nov 19, 1999 at 05:38:16AM +0000, Andrew Gierth wrote:
...
> Specifically, CommaSplit(",") should return { "", "", NULL }.
> 
> The bug, I suspect, is really here:
> 
> 	if (ME.Distributions
> 	 && !DISTwantany(ME.Distributions, distributions)) {
> 	    (void)sprintf(buff, "%d Unwanted distribution \"%s\"",
> 		    NNTP_REJECTIT_VAL,
> 		    MaxLength(distributions[0], distributions[0]));
> 
> because DISTparse can cause distributions[0] to be NULL if all the
> fields returned from CommaSplit are empty, and MaxLength (prior to
> your other patch) explodes if passed a NULL pointer.

  This is actually where it blew up and I caught it in gdb, yes.

  I wasn't about to try to set up a test-rig where I could feed that
case through innd and test it in situ, but I made the wrong assumption
about where the program went astray.

  I hadn't read through it enough to realize that DISTparse would
actually rewrite the contents of distributions[] as a side-effect, so I
assumed that distributions[0] was also NULL before the call.

  Hmmm, if DISTparse is taking a non-null pointer, and returning a null
pointer, is that a possible memory leak?


> Notice that this only kicks in if ME.Distributions is set; if you have
> no distributions on your ME line then nothing bad seems to happen.

  Yes, I considered dumping distributions out of our conf file as well,
but decided it was safer to fix the source.  (It's a good thing I
thought it prudent to fix MaxLength as well.)

  
> I'd suggest either
> 
> 	if (ME.Distributions
> +        && distributions[0]

  That looks good.  I'll roll it in.


> 	 && !DISTwantany(ME.Distributions, distributions)) {
> 	    (void)sprintf(buff, "%d Unwanted distribution \"%s\"",
> 		    NNTP_REJECTIT_VAL,
> 		    MaxLength(distributions[0], distributions[0]));
>       
> which will treat an article with no non-empty distributions as a
> wanted article, or just use your patch to MaxLength if you prefer
> to reject such articles.

  The patch to MaxLength is important on its own, because it appears to
be called for logging in a variety of error cases, and there's
therefore a reasonable likelihood of a bug somewhere else triggering a
call to MaxLength with a null pointer.

  If I could (humbly) suggest some variation of that patch for the inn
mainstream, please consider it.

> 
> -- 
> Andrew.

-- 
 Clifton Royston  --  LavaNet Systems Architect --  cliftonr at lava.net
        "An absolute monarch would be absolutely wise and good.  
           But no man is strong enough to have no interest.  
             Therefore the best king would be Pure Chance.  
              It is Pure Chance that rules the Universe; 
          therefore, and only therefore, life is good." - AC


More information about the inn-bugs mailing list