cleaned up abit more and put in the use of force params, although im not
sure using them is the correct thing
cant see a way of writing a test for this though unfortunatlly
2011/8/4 Keloran <ava...@gmail.com>
> 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
