Re: [PHP-DEV] [PATCH] Constification of some PHP API calls

[Andrey Hristov]

On 07/31/2013 04:56 PM, Sara Golemon wrote:

Ah, yeah, I can see that clearly now.  Looks universally cool to me.  Do
you just need someone with engine karma to push it?


right. I have no Zend karma, or at least didn't have at CVS/SVN times.




On Wed, Jul 31, 2013 at 1:35 AM, Andrey Hristov  wrote:


  Hi Sara,

On 07/31/2013 12:55 AM, Sara Golemon wrote:


At a glance, this looks awesome.  There are a few changes where you
introduced #ifdef WIN32 checks that I want to spend a little more time
looking at...



In the WIN32 code there was an additional variable, declared always, but
switched from 0 to 1 only in under WIN32. It is a path, that is copied and
the copy is assigned to the parameter. I have simplified the logic for all
platforms by putting more WIN32 conditionals.



  -Sara


P.S. - Could I convince you to put it on a github fork to make it easier
to read?  No big deal if not... I just like that web view. :)



I have already done that :
https://github.com/**andreyhristov/php-src

I have filled a pull request too.

Best,
Andrey



On Mon, Jul 29, 2013 at 3:01 AM, Andrey Hristov mailto:p...@hristov.com>> wrote:

   Hi,
 after experiencing again warnings about conversion from const char *
 to char * when calling some PHP API functions I decided to spend
 some time and constify a few of these. The result is a patch that
 constifies the Streams API, and few functions in Zend and TSRM (to
 which I have no karma). Most of the changes are conversion of "char
 *" to "const char *" parameters, where applicable. In a few places
 string lengths are changed from int to size_t. Few functions return
 now "const char *" instead of "char *" (the caller did not
 modify/release these before).

 The patch is against php-src because it changes API which can be
 only done in a new version.

 Opinions?

 Best,
 Andrey

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










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

Re: [PHP-DEV] [PATCH] Constification of some PHP API calls

[Sara Golemon]

Ah, yeah, I can see that clearly now.  Looks universally cool to me.  Do
you just need someone with engine karma to push it?


On Wed, Jul 31, 2013 at 1:35 AM, Andrey Hristov  wrote:

>  Hi Sara,
>
> On 07/31/2013 12:55 AM, Sara Golemon wrote:
>
>> At a glance, this looks awesome.  There are a few changes where you
>> introduced #ifdef WIN32 checks that I want to spend a little more time
>> looking at...
>>
>
> In the WIN32 code there was an additional variable, declared always, but
> switched from 0 to 1 only in under WIN32. It is a path, that is copied and
> the copy is assigned to the parameter. I have simplified the logic for all
> platforms by putting more WIN32 conditionals.
>
>
>
>  -Sara
>>
>> P.S. - Could I convince you to put it on a github fork to make it easier
>> to read?  No big deal if not... I just like that web view. :)
>>
>
> I have already done that :
> https://github.com/**andreyhristov/php-src
>
> I have filled a pull request too.
>
> Best,
> Andrey
>
>
>> On Mon, Jul 29, 2013 at 3:01 AM, Andrey Hristov > > wrote:
>>
>>   Hi,
>> after experiencing again warnings about conversion from const char *
>> to char * when calling some PHP API functions I decided to spend
>> some time and constify a few of these. The result is a patch that
>> constifies the Streams API, and few functions in Zend and TSRM (to
>> which I have no karma). Most of the changes are conversion of "char
>> *" to "const char *" parameters, where applicable. In a few places
>> string lengths are changed from int to size_t. Few functions return
>> now "const char *" instead of "char *" (the caller did not
>> modify/release these before).
>>
>> The patch is against php-src because it changes API which can be
>> only done in a new version.
>>
>> Opinions?
>>
>> Best,
>> Andrey
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>
>>
>

Re: [PHP-DEV] [PATCH] Constification of some PHP API calls

[Andrey Hristov]

 Hi Sara,
On 07/31/2013 12:55 AM, Sara Golemon wrote:

At a glance, this looks awesome.  There are a few changes where you
introduced #ifdef WIN32 checks that I want to spend a little more time
looking at...


In the WIN32 code there was an additional variable, declared always, but 
switched from 0 to 1 only in under WIN32. It is a path, that is copied 
and the copy is assigned to the parameter. I have simplified the logic 
for all platforms by putting more WIN32 conditionals.




-Sara

P.S. - Could I convince you to put it on a github fork to make it easier
to read?  No big deal if not... I just like that web view. :)


I have already done that :
https://github.com/andreyhristov/php-src

I have filled a pull request too.

Best,
Andrey



On Mon, Jul 29, 2013 at 3:01 AM, Andrey Hristov mailto:p...@hristov.com>> wrote:

  Hi,
after experiencing again warnings about conversion from const char *
to char * when calling some PHP API functions I decided to spend
some time and constify a few of these. The result is a patch that
constifies the Streams API, and few functions in Zend and TSRM (to
which I have no karma). Most of the changes are conversion of "char
*" to "const char *" parameters, where applicable. In a few places
string lengths are changed from int to size_t. Few functions return
now "const char *" instead of "char *" (the caller did not
modify/release these before).

The patch is against php-src because it changes API which can be
only done in a new version.

Opinions?

Best,
Andrey

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





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

Re: [PHP-DEV] [PATCH] Constification of some PHP API calls

[Sara Golemon]

At a glance, this looks awesome.  There are a few changes where you
introduced #ifdef WIN32 checks that I want to spend a little more time
looking at...

-Sara

P.S. - Could I convince you to put it on a github fork to make it easier to
read?  No big deal if not... I just like that web view. :)


On Mon, Jul 29, 2013 at 3:01 AM, Andrey Hristov  wrote:

>  Hi,
> after experiencing again warnings about conversion from const char * to
> char * when calling some PHP API functions I decided to spend some time and
> constify a few of these. The result is a patch that constifies the Streams
> API, and few functions in Zend and TSRM (to which I have no karma). Most of
> the changes are conversion of "char *" to "const char *" parameters, where
> applicable. In a few places string lengths are changed from int to size_t.
> Few functions return now "const char *" instead of "char *" (the caller did
> not modify/release these before).
>
> The patch is against php-src because it changes API which can be only done
> in a new version.
>
> Opinions?
>
> Best,
> Andrey
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

Re: [PHP-DEV] [PATCH] Constification of some PHP API calls

[Julien Pauli]

On Mon, Jul 29, 2013 at 12:01 PM, Andrey Hristov  wrote:

>  Hi,
> after experiencing again warnings about conversion from const char * to
> char * when calling some PHP API functions I decided to spend some time and
> constify a few of these. The result is a patch that constifies the Streams
> API, and few functions in Zend and TSRM (to which I have no karma). Most of
> the changes are conversion of "char *" to "const char *" parameters, where
> applicable. In a few places string lengths are changed from int to size_t.
> Few functions return now "const char *" instead of "char *" (the caller did
> not modify/release these before).
>
> The patch is against php-src because it changes API which can be only done
> in a new version.
>
> Opinions?
>

I'm +1.

About strings sizes from int to size_t , you perhaps could sync with
Anthony who worked hard recently on such changes, especially regarding zval
string type.

Julien.Pauli