[PATCH] Constification of some PHP API calls

13 years ago by Andrey Hristov — view source — reply

unread

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

13 years ago by Julien Pauli — view source — reply

unread

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

13 years ago by Sara Golemon — view source — reply

unread

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. :)

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

13 years ago by Andrey Hristov — view source — reply

unread

Hi Sara,

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 <php@hristov.com
mailto:php@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

13 years ago by Sara Golemon — view source — reply

unread

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

Hi Sara,

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 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 <php@hristov.com
mailto:php@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

13 years ago by Andrey Hristov — view source — reply

unread

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.

Hi Sara,

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 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 <php@hristov.com
mailto:php@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