By the way, very simple fix:

Add this to the variable declarations:

int j = 0;

Then the lines of code as mentioned before, but fixed:

        if (to_len > 0) {
                to_r = estrndup(to, to_len);

                for (j = to_len; j > 0; j--) {
                        if (!isspace((unsigned char) to_r[j - 1])) {
                                break;
                        }

                        to_r[j - 1] = '\0';
                }



and:

        if (subject_len > 0) {
                subject_r = estrndup(subject, subject_len);

                for (j = subject_len; j > 0; j--) {
                        if (!isspace((unsigned char) subject_r[j - 1])) {
                                break;
                        }

                        subject_r[j - 1] = '\0';
                }


I just initialized j in the for loop to be the value of to_len and subject_len, then I use j everywhere rather than to_len or subject_len so that they stay unmodified.

I guess an alternate fix would also be when the efree()'s are called.  If you init all your char *'s to NULL, then you can simply do:

if (to_r != NULL) {
     efree(to_r);
}

if (subject_r != NULL) {
     efree(subject_r);
}

Impossible leak. 
        if (to_len > 0) { 
                efree(to_r); 
        } 
        if (subject_len > 0) { 
                efree(subject_r); 
        } 
Is what the code in CVS does. If to_len or subject_len are 
< 1 then no allocation happens in the 1st place. 

I know they check to_len and subject_len - that's not really the problem.

The problem is that the for loops above that decrement to_len and subject_len - thus modifying them from their original values.

to_len and subject_len will always be 0, even if they weren't 0 to begin with.  Do you see what I'm referring to?

Here, maybe this will help a bit...  Here it assigns values to to_len and subject_len (among others):

        if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sss|ss",
                                                          &to, &to_len,
                                                          &subject, &subject_len,
                                                          &message, &message_len,
                                                          &headers, &headers_len,
                                                          &extra_cmd, &extra_cmd_len
                                                          ) == FAILURE) {
                return;
        }


Then, they check to_len and do stuff if it's greater than 0:

        if (to_len > 0) {
                to_r = estrndup(to, to_len);
                for (; to_len; to_len--) {
                        if (!isspace((unsigned char) to_r[to_len - 1])) {
                                break;
                        }
                        to_r[to_len - 1] = '\0';
                }
...


Do you see the for loop in there... this one:

                for (; to_len; to_len--) {...}

It is modifying to_len itself, which means that to_len, although was NOT 0 to begin with (and thus, to_r was estrndup()'d and we have to efree() it later), but IS 0 in the end once the for loop is finished.

Either the for loop must be changed to not modify to_len, or the efree() statement must be changed to test to_r, not to_len.

Or am I just really out of my mind?  I'm not anywhere near as good as you programmers, but this seems to be sticking out for me quite a bit (I'm not trying to come off rude!  I just think I found something here and it's not bogus).

Ok, maybe I am wrong?

I wrote a PHP script which *should* leak memory if this is indeed not efree()'ing stuff, but it doesn't seem to.

I noticed it would only potentially (if I was right!) leak ram if say, the subject was entirely space characters, so I made a script that called mail() 1000 times roughly, and made a subject that was all spaces that was 1k long - it should have set me off by 1mb, but instead I see nothing of the sort.

I'm running the script from the command line (CGI, not CLI though!) if it makes any difference (I don't believe it does).

Either way, I can't seem to leak the ram, so I guess I was wrong - but can someone explain to me why it wouldn't?  What am I missing here that wouldn't allow it to leak ram?

This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

The bug was real, you were right. The reason you did not 
see the leak is because Zend's memory manager automatically 
handles such leaks. If you were to compile a debug build of 
PHP you'd see some leak messages printed.