Re: [bug-mailutils] look this and think, is this a bug!

[lhffjzh]

Hi Sergey:

    I read you skills on Savannah, very admire you. thanks for give this reply!

    But I consider that code may have problem, as follows, my comment is start with "//", original comment with "/* */".

    I invoke function with mu_mailbox_create_default(pmbox, "fill://home/haifeng/mailutils/clean") then

int
mu_mailbox_create_default (mu_mailbox_t *pmbox, const char *mail)
{
  char *mbox = NULL;
  char *tmp_mbox = NULL;
  char *p;
  int status = 0;

  /* Sanity.  */
  if (pmbox == NULL)
    return MU_ERR_OUT_PTR_NULL;

  /* Other utilities may not understand GNU mailutils url namespace, so
     use FOLDER instead, to not confuse others by using MAIL.  */

  //mail = "fill://home/haifeng/mailutils/clean", then this isn't invoked, tmp_mbox is NULL
  if (mail == NULL || *mail == '\0')
  {
      mail = getenv ("FOLDER");

      /* Fallback to wellknown environment.  */
      if (!mail)
        mail = getenv ("MAIL");

      if (!mail)
      {
          if ((status = user_mailbox_name (NULL, &tmp_mbox)))
            return status;
          mail = tmp_mbox;
      }
  }

  //after invoke mu_tilde_expansion, p point a new memory by function malloc(strdup), but tmp_mbox is NULL, then "tmp_mbox = p;" can't be invoked, so finaly "if (tmp_mbox) free(tmp_mbox)", but not free pointer p. I think that can modify that code to right one.
  p = mu_tilde_expansion (mail, "/", NULL);
  if (tmp_mbox)                           //if (tmp_mbox)
  {                                       //    free(tmp_mbox)   
      free (tmp_mbox);                    //
      tmp_mbox = p;                       //tmp_mbox = p;
  }                                       //
  mail = p;
  if (!mail)
    return ENOMEM;
 
  switch (mail[0])
    {
    case '%':
      status = percent_expand (mail, &mbox);
      break;
     
    case '+':
    case '=':
      status = plus_expand (mail, &mbox);
      break;

    case '/':
      mbox = strdup (mail);
      break;
     
    default:
      if (!mu_is_proto (mail))
 {
   tmp_mbox = mu_getcwd();
   mbox = malloc (strlen (tmp_mbox) + strlen (mail) + 2);
   sprintf (mbox, "%s/%s", tmp_mbox, mail);
 }
      else
 mbox = strdup (mail);
      break;
    }

  if (tmp_mbox)
    free (tmp_mbox);

  if (status)
    return status;
 
  status = mu_mailbox_create (pmbox, mbox);
  free (mbox);
  return status;

}

----- Original Message -----
From: "Sergey Poznyakoff" <address@hidden>
To: <address@hidden>
Cc: <address@hidden>
Sent: Friday, April 13, 2007 12:33 AM
Subject: Re: [bug-mailutils] look this and think, is this a bug!


> <address@hidden> wrote:
>
>>     I read some code of mailbox,  there is a function name
>>     mu_mailbox_create_default  as follows,  i add a comment and a few
>>     codes,  then resolve a memory leak
>
> There are two possible memory leaks in this function.  They are fixed in
> the repository.  Thanks for reporting.
>
>>  /* I add comment and a few codes */
>>  /* if ((mail != NULL) && (tmp_mbox == NULL)), then char *p not free, then add this */
>>   if (p)
>>     free (p);
>
> This change thrashes the memory area and coredumps right away (in the
> best case), or induces a coredump in some remote point (in the worst
> case).  Both `p' and `tmp_mbox' can point to the same memory location,
> which will be freed twice.
>
> Regards,
> Sergey
>