potential memory leaks

potential memory leaks

[Gary Yee]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

Hi!
I'm currently looking over the code for version 
3.113 of GNATS.  I think there are some potential memory 
leaks.
 
Here is some code from main.c:
 
  /* Check if the message is a reply to an 
existing PR; if it is, then
     just append this message to 
the end of that PR. Otherwise, process
     it normally. */
  pr_path = check_if_reply ();
  if (pr_path)
    {
    /* Before inserting the reply, see 
if someone's got the PR
       locked; if 
they do, then try it at the next run of the queue. */
      char *buf = 
get_lock_path (pr_path);
      if (access (buf, R_OK) 
== 0)
        
{
          
unlock_gnats ();
          
exit (1);
        
}
      append_report (fp, 
pr_path);
      xfree 
(pr_path);
    }
  else
    gnats (fp);
 
  /* Clean up. */
  if (log_method == MAIL)
    close_mail_file 
(gnats_logfile);
#ifdef HAVE_SYSLOG_H
  else if (log_method == 
SYSLOG)
    closelog ();
#endif
 
  /* unlock the whole thing. */
  unlock_gnats ();
  free_db_conf();
  exit (0);
}
 
If the call to access() returns 0, which means that 
the PR is already locked, shouldn't there be calls to close_mail_file(), 
free_db_conf(), and xfree() along with the call to unlock_gnats()?
 
In headers.c, the call to set_continued_header() is 
immediately followed by the line
 
    buf = NULL;
 
This assumes that the memory originally pointed to 
by buf gets assigned to something in the set_continued_header function.  
This is true except when header[i].value is not NULL and (keep_rec && i 
== RECEIVED) is false.  In this case, nothing is done with 
buf.
 
That's all for now.
 
Regards,
Gary
 

Re: potential memory leaks

[Rick Macdonald]

On Wed, 12 Jan 2000, Gary Yee wrote:

> Hi! I'm currently looking over the code for version 3.113 of GNATS.  
> I think there are some potential memory leaks.

There are probably a _lot_ of memory leaks. GNATS V4 is nearly ready for a
first release. It's a huge overhaul and a lot of code has changed. So,
memory leaks in v3.x aren't of much interest these days.

Besides, there aren't any gnats programs that run long enough or keep
gnatsd connected long enough for to see a problem from those leaks anyway,
I figure. 

V4 will be worth keeping memory-clean, but V3 is soon to be history.

...RickM...

Re: potential memory leaks

[Bob Manson]

I've been carfully checking the v4 code, and I believe there aren't
any leaks.  (I've been using several different tools to analyze this
and they all concur, tho my code coverage isn't 100% yet--checking
some of the error cases is a bit tricky.)  So no real worries there.

And as Rick points out, none of the programs actually hang around long
enough that it really matters.  However, if something like query-pr
sucks in a lot of PRs and it's leaking memory in the PR-reading code,
that's a a bit of a problem.  Or if there's something in the server
that can be triggered and someone wants to do a DOS attack or just be
nasty, that's also a problem.

The last set of changes to go in to 4.0 remove all line-length limits
(depending on how much memory you have...) I've successfully worked
with PRs that have 4mb-long lines, which is good 'nough for me.  I now
need to cook up a testsuite that exercises most of the code and make
sure the docs are up-to-date.
						Bob