Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:69472
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain fedoraproject.org from 212.27.42.4 cause and error)
Message-ID: <524CEA7E.5070107@fedoraproject.org>
Date: Thu, 03 Oct 2013 05:54:38 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0
MIME-Version: 1.0
To: PHP Internals <internals@lists.php.net>
References: <CAEKnhAEM+XijRRqjf6EpJBKHvxyShP3bHQ5E7CCcK6_MbFwHpg@mail.gmail.com>
In-Reply-To: <CAEKnhAEM+XijRRqjf6EpJBKHvxyShP3bHQ5E7CCcK6_MbFwHpg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Streams constify API change in master leads to compilation
 warning for extensions
From: remi@fedoraproject.org (Remi Collet)

Le 02/10/2013 20:41, Jakub Zelenka a écrit :
> Hi,
> 
> I was wondering why stream API has been changed in this commit:
> 
> https://github.com/php/php-src/commit/92d27ccb0574f901a107409a7fec92888fa2b82f
> 
> Basically all char pointers have been constified. The thing is that this
> commit leads to compilation warning for many extensions.
> 
> In my case I use php_stream_locate_url_wrapper and want to compile my
> extension (fann) without any warnings. Because this change is in master and
> not in 5.5-, I will have to add some ifdefs and cast it for 5.6+.
> 
> The thing is that php_stream_locate_url_wrapper and other stream fuctions
> are often used for function parameters from zend_parse_parameters which are
> just char *. Then the values have to be casted.
> 
> I understand that APIs should be improved  and sometimes changes are
> necessary but in this case I have to ask a question. Was this change worthy
> to compilation warnings for many extensions?

I don't see any problem with this change.

When a (char *) is expected (which means, give me a pointer to something
I can change), you can only give a (char *). So a (const char *) raises
a warning.

When a (const char *) is expected (which means, give me a pointer to
something I won't change), you can give a (char *) or a (const char *),
both are accepted without any warning.

In fann you had a (char *),
I propose you to remove the cast to (const char *) which raise a warning
with 5.5 and is not useful in master.

In the opposite way a cast from const to not-const have of course no sense.

So, yes we need to add more const in php API, as this is the correct way
to properly fix various build warnings.

About the linked commit, I think it would be useful in 5.5 (as this
don't break ABI).

Note, a string (such as "some name") is consider by gcc as a (char *)
but as a (const char *) by g++ (except if -fno-const-strings is used).

Ex :
http://git.php.net/?p=php-src.git;a=commitdiff;h=f7eff9cd41e0b996af9a0a01d3c5f8fdd8b7fa60
This constify option of exception functions fixes warning (at least) in
xmldiff extension (written in C++) which use const strings.


Remi.

PS. is a warning free build a dream ? I don't think.
> 
> If not would it be possible to revert it?
> 
> Thanks
> 
> Jakub
>