[PATCH v3] wayland-util: added wl_strtol/wl_strtoul utility functions

[Giulio Camuffo]

2014-10-29 8:45 GMT+02:00 Imran Zaman <imran.zaman at gmail.com>:
> Daniel!
>
> As per your logic, I see wl_list APIs exposed etc, which shouldn't be part
> of libwayland as well.
> similarly, wl_fixed_to_double and wl_array shouldn't be part of the window
> system. Isnt it?
> I can make inline functions if that helps.

wl_list is used in the server side API, so it's a bit different.
However, I'd agree that it'd be better if it wasn't exposed but we
can't remove it now. wl_fixed is a wayland specific type so all the
wl_fixed_* functions need to be part of the API.
On the other hand wl_strtol would just be a function, there are is no
other API that depends on it.

>
> Btw here is an API patch, which has not be reviewed till now.
> http://lists.freedesktop.org/archives/wayland-devel/2014-October/017833.html

Yes, like there are many other patches waiting for reviews. You need
to have patience, it's not like we are ignoring it. But, if I may add,
the way you are reacting to a comment to this patch doesn't encourage
to review your other ones.


--
Giulio

>
> BR
> imran
>
> On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone <daniel at fooishbar.org> wrote:
>
>> Hi,
>>
>> On 28 October 2014 15:40, Imran Zaman <imran.zaman at gmail.com> wrote:
>>>
>>> You guys should check the reason why the patch is there rather than
>>> throwing out random thoughts or blunt comments.
>>>
>>> I did this patch mainly because weston/wayland has been using
>>> strtol/strtoul functions in number of places with buggy error checks, and
>>> duplicate code everywhere. Weston and wayland go together; so in bigger
>>> picture, its a very useful patch IMO.. I hardly find any patches with proper
>>> tests, but I did it so to make it more effective. But I guess in
>>> wayland/weston community, only maintainers are allowed to push patches
>>> others are strongly discouraged to do so. I guess its better to encourage
>>> people/community for giving helping hand.
>>>
>>> Anyways we will now only push patches (including multi-seat support) in
>>> Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as
>>> it seems to be long bureaucratic process to overcome with virtually no
>>> success.
>>
>>
>> That's not what we've said. No-one said 'don't take the patch'; all we
>> said is 'please don't expose it as part of libwayland-*'s _public_ API'.
>>
>> I like the idea of the patch, I like how it's caught a number of buggy
>> spots where we've open-coded the same thing, and I like that it's
>> well-tested. For internal usage, it's great! I just don't want to expose
>> string manipulation functions in the public API of a window system.
>>
>> You're right that the test infrastructure is in a pretty bad state.
>> Anything which helps that is more than welcome, and you may have seen a
>> bunch of patches from Derek Foreman (not a maintainer) on this list, which
>> are progressing well and go a long way towards improving our test suite into
>> something that will be really useful day to day. Any further contributions
>> along those lines - from anyone - are totally welcome.
>>
>> As for your multiseat patches, the last discussions I remember involved
>> some pretty fundamental disagreements about the whole architecture,
>> particularly input support. I haven't seen any more patches or discussion
>> since the last IRC chat, though.
>>
>> Cheers,
>> Daniel
>>
>
> On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone <daniel at fooishbar.org> wrote:
>>
>> Hi,
>>
>> On 28 October 2014 15:40, Imran Zaman <imran.zaman at gmail.com> wrote:
>>>
>>> You guys should check the reason why the patch is there rather than
>>> throwing out random thoughts or blunt comments.
>>>
>>> I did this patch mainly because weston/wayland has been using
>>> strtol/strtoul functions in number of places with buggy error checks, and
>>> duplicate code everywhere. Weston and wayland go together; so in bigger
>>> picture, its a very useful patch IMO.. I hardly find any patches with proper
>>> tests, but I did it so to make it more effective. But I guess in
>>> wayland/weston community, only maintainers are allowed to push patches
>>> others are strongly discouraged to do so. I guess its better to encourage
>>> people/community for giving helping hand.
>>>
>>> Anyways we will now only push patches (including multi-seat support) in
>>> Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as
>>> it seems to be long bureaucratic process to overcome with virtually no
>>> success.
>>
>>
>> That's not what we've said. No-one said 'don't take the patch'; all we
>> said is 'please don't expose it as part of libwayland-*'s _public_ API'.
>>
>> I like the idea of the patch, I like how it's caught a number of buggy
>> spots where we've open-coded the same thing, and I like that it's
>> well-tested. For internal usage, it's great! I just don't want to expose
>> string manipulation functions in the public API of a window system.
>>
>> You're right that the test infrastructure is in a pretty bad state.
>> Anything which helps that is more than welcome, and you may have seen a
>> bunch of patches from Derek Foreman (not a maintainer) on this list, which
>> are progressing well and go a long way towards improving our test suite into
>> something that will be really useful day to day. Any further contributions
>> along those lines - from anyone - are totally welcome.
>>
>> As for your multiseat patches, the last discussions I remember involved
>> some pretty fundamental disagreements about the whole architecture,
>> particularly input support. I haven't seen any more patches or discussion
>> since the last IRC chat, though.
>>
>> Cheers,
>> Daniel
>
>