[PATCH 2/2] wayland-util: Rename ARRAY_LENGTH -> __WL_ARRAY_LENGTH

Kristian Høgsberg krh at bitplanet.net
Fri Oct 19 20:33:30 PDT 2012


On Thu, Oct 18, 2012 at 6:52 PM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> On 10/18/2012 01:48 PM, Kristian Høgsberg wrote:
>> On Thu, Oct 18, 2012 at 4:27 PM, David Herrmann
>> <dh.herrmann at googlemail.com> wrote:
>>> Hi Chad
>>>
>>> On Thu, Oct 18, 2012 at 9:45 PM, Chad Versace
>>> <chad.versace at linux.intel.com> wrote:
>>>> On 10/18/2012 12:35 PM, Chad Versace wrote:
>>>>> On 10/18/2012 10:23 AM, Pekka Paalanen wrote:
>>>>>> On Thu, 18 Oct 2012 09:15:08 -0700
>>>>>> Chad Versace <chad.versace at linux.intel.com> wrote:
>>>>>>
>>>>>>> wayland-util.h defined an unprefixed macro, ARRAY_LENGTH, which polluted
>>>>>>> the global namespace. This caused symbol collisions in projects that
>>>>>>> defined ARRAY_LENGTH slightly differently.
>>>>>>>
>>>>>>> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
>>>>>>
>>>>>> Hi Chad,
>>>>>>
>>>>>> do you have the weston patches to go with these?
>>>>>> I think Weston code heavily uses both the macros you replaced.
>>>>>
>>>>> No, I wasn't aware that the macros were used outside Wayland. The
>>>>> Weston patches coming soon.
>>>>
>>>> Hmm... I think the ARRAY_LENGTH needs to be revised. Since
>>>> wayland-util.h:ARRAY_LENGTH is used outside of Wayland, I think it should be
>>>> renamed to WL_ARRAY_LENGTH rather than __WL_ARRAY_LENGTH because
>>>> double-underscore symbols are typically private.
>>>>
>>>> Opinions?
>>>
>>> I think adding them to weston separately is ok. They are really not
>>> related to wayland at all so I don't understand why we should make it
>>> part of the libwayland API.
>>
>> Yeah, that's better.  We can just move ARRAY_LENGTH to
>> wayland-private.h and redefine it in weston where we need it.  As for
>> container_of, we use it in the wl_list_foreach macros, so lets keep
>> that and call it wl_container_of so we don't have to worry about who
>> owns the __* namespace.
>
>
> Now I'm confused about container_of.
>
> The wl_list_foreach macros use __wl_container_of, not container_of. So, I see
> three ways to interpret your suggestion:
>
>   1. - Rename container_of to wl_container_of.
>      - Change nothing else. This leaves the __wl_container_of and
>        wl_list_foreach* macros untouched.
>
>   2. - Rename container_of -> wl_container_of.
>      - Replace all use of __wl_container_of with wl_container_of.
>      - Remove __wl_container_of.
>
>   3. - Rename __wl_container_of to wl_container_of.
>      - Replace all use of container_of with wl_container_of.
>      - Remove container_of.
>
> I prefer (3), but it's not really my project. Just let me know which you want
> and I'll rewrite my patch.

Thanks Chad - I ended up just making the change myself.  What I did
was to keep using container_of, but move it into private headers in
wayland and weston.  __wl_container_of causes undefined behavior if
the variable passed as the 'sample' argument  isn't initialized.
container_of avoids this by taking a struct type there instead and I'd
like to keep using that.  Then I renamed __wl_container_of to just
wl_container_of, since __* identifiers are reserved.

Kristian


More information about the wayland-devel mailing list