On Thu, Oct 18, 2012 at 6:52 PM, Chad Versace <[email protected]> wrote: > On 10/18/2012 01:48 PM, Kristian Høgsberg wrote: >> On Thu, Oct 18, 2012 at 4:27 PM, David Herrmann >> <[email protected]> wrote: >>> Hi Chad >>> >>> On Thu, Oct 18, 2012 at 9:45 PM, Chad Versace >>> <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>>>> >>>>>> 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
