smart_str_appendl() would be faster since it'll avoid having to calculate the string length, and sizeof() can be used to determine the length of the constant, resulting in faster code.

On a general note, I am not sure its a good idea to change the EOL for CSV file, since many scanners expect "\n" as a line separator and the change may introduce regressions.


On 22-Oct-08, at 4:26 PM, John Mertic wrote:

Hi Ilia,

Sorry for my C confusion, like I said it's been awhile.

Anyways, would smart_str_appends() be the correct function to use then?

John Mertic
[EMAIL PROTECTED]
http://jmertic.wordpress.com

"Explaining a joke is like dissecting a frog: you understand it
better, but the frog dies
in the process." --Mark Twain



On Wed, Oct 22, 2008 at 4:16 PM, Ilia Alshanetsky <[EMAIL PROTECTED]> wrote:
You cannot use smart_str_appendc() in this case, since the EOL could be a 2
byte string "\r\n".

smart_str_appendl(&csvline, PHP_EOL, sizeof(PHP_EOL)-1); should be used
here.

On 22-Oct-08, at 2:35 PM, John Mertic wrote:

Hi Lars,

Thanks for the pointers, updated the patch and added a test.

Index: file.c
===================================================================
RCS file: /repository/php-src/ext/standard/file.c,v
retrieving revision 1.530
diff -u -r1.530 file.c
--- file.c      21 Oct 2008 22:06:48 -0000      1.530
+++ file.c      22 Oct 2008 18:21:10 -0000
@@ -2104,7 +2104,7 @@
              }
      }

-       smart_str_appendc(&csvline, '\n');
+       smart_str_appendc(&csvline, PHP_EOL);
      smart_str_0(&csvline);

      ret = php_stream_write(stream, csvline.c, csvline.len);


John Mertic
[EMAIL PROTECTED]
http://jmertic.wordpress.com

"Explaining a joke is like dissecting a frog: you understand it
better, but the frog dies
in the process." --Mark Twain



On Wed, Oct 22, 2008 at 10:36 AM, Lars Strojny <[EMAIL PROTECTED]> wrote:

Hi John,


Am Mittwoch, den 22.10.2008, 10:03 -0700 schrieb John Mertic:
[...]

Below is a patch to fix this issue; it uses the constant PHP_EOL to
get the correct newline to use on the current platform:

Thanks for your patch. A few things to mention, as it is your first
patch: please use "diff -ru" to create unified diffs. Also we try to
always add tests for the things we fix or create. Would you mind
creating a test for the fix you sent to make sure no regression happens
in the next n years?

Thanks, Lars
--
Jabber: [EMAIL PROTECTED]
Weblog: http://usrportage.de


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Ilia Alshanetsky






Ilia Alshanetsky





--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to