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
