Yeah it does need cleaning up, didn't know about the new mail options,
main reason I did it was because the function has had the option since
2004, but it's never actually used it
On Thursday, August 4, 2011, Johannes Schlüter <johan...@schlueters.de> wrote:
> Hi,
>
> few comments just from reading the patch, not the full context, not
> testing it:
>
> On Thu, 2011-08-04 at 16:30 +0100, Keloran wrote:
>> Index: ext/imap/php_imap.c
>> ===================================================================
>> --- ext/imap/php_imap.c (revision 314217)
>> +++ ext/imap/php_imap.c (working copy)
>> @@ -4016,7 +4016,27 @@
>> if (!INI_STR("sendmail_path")) {
>> return 0;
>> }
>> - sendmail = popen(INI_STR("sendmail_path"), "w");
>> +
>> + /** Used to make return-path work **/
>> + char *sendmail_path = INI_STR("sendmail_path");
>> + char *appended_sendmail_path = NULL;
>
> This won't work on all compiles, declarations have to go first in the
> block.
>
>> + /** Return Path or not **/
>> + if (rpath && rpath[0]) {
>> + appended_sendmail_path = emalloc(
>> + strlen(sendmail_path) + 3 + strlen(rpath) + 1);
>> + strcpy(appended_sendmail_path, sendmail_path);
>> + strcat(appended_sendmail_path, " -f");
>> + strcat(appended_sendmail_path, rpath);
>> + sendmail_path = appended_sendmail_path;
>> + }
>> +
>> + /** open the sendmail pointer **/
>> + sendmail = popen(sendmail_path, "w"); /* New Code */
>
> How do you open a pointer ;-)
> I think everybody reading the code should know popen() so the first
> comment isn't really useful; I assume the second comment there was for
> your purpose.
>
>> + if (appended_sendmail_path)
>> + efree(appended_sendmail_path);
>> +
>
> Always use { } even for a single line (see CODING_STANDARDS)
>
> While editing this functions: Should this function not also respect the
> new mail.force_extra_parameters and mail.log settings, too? (Might need
> an extra bug report to be reported correctly ...)
>
> johannes
>
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
