Mir

mir_toolkit namespace around client API functions is pointless

Bug #1137184 reported by Daniel van Vugt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
Undecided
Alan Griffiths
Mir 0.0.3

Bug Description

Recently the mir_toolkit namespace was added and it was applied to client API functions too:

    src/client/mir_client_library.cpp:
    mir_toolkit::MirWaitHandle* mir_toolkit::mir_connect(...

Although the client header says 'extern "C"' so the namespace is not emitted in the compiled output:

    nm lib/libmirclient.so.0 | grep 'T mir_'
    0000000000030840 T mir_connect

This means all the messiness of mentioning "mir_toolkit::" in mir_client_library.cpp appears to be pointless. And it has also caused bug 1137170 too.

Related branches

lp://qastaging/~alan-griffiths/mir/fix-bug-1137170
PS Jenkins bot (community): Approve (continuous-integration)
Daniel van Vugt: Needs Fixing
Kevin DuBois (community): Approve
Alexandros Frantzis (community): Approve
Diff: 1313 lines (+170/-164)
35 files modified
include/client/mir_toolkit/mir_client_library.h (+4/-5)
include/client/mir_toolkit/mir_client_library_drm.h (+5/-2)
include/client/mir_toolkit/mir_client_library_lightdm.h (+5/-2)
include/shared/mir_toolkit/c_types.h (+5/-2)
include/shared/mir_toolkit/common.h (+6/-1)
include/shared/mir_toolkit/input/event.h (+5/-0)
include/shared/mir_toolkit/mesa/native_display.h (+5/-3)
src/client/android/android_client_buffer_factory.cpp (+1/-1)
src/client/android/android_client_buffer_factory.h (+1/-1)
src/client/android/android_native_display_container.cpp (+6/-6)
src/client/android/android_native_display_container.h (+3/-3)
src/client/client_buffer.h (+6/-4)
src/client/client_buffer_depository.cpp (+1/-1)
src/client/client_buffer_depository.h (+1/-6)
src/client/client_buffer_factory.h (+1/-1)
src/client/client_context.h (+2/-8)
src/client/egl_native_display_container.h (+3/-3)
src/client/gbm/gbm_client_buffer.cpp (+2/-2)
src/client/gbm/gbm_client_buffer.h (+3/-3)
src/client/gbm/gbm_client_buffer_factory.cpp (+1/-1)
src/client/gbm/gbm_client_buffer_factory.h (+1/-1)
src/client/gbm/gbm_client_platform.cpp (+1/-1)
src/client/gbm/mesa_native_display_container.cpp (+9/-9)
src/client/gbm/mesa_native_display_container.h (+4/-4)
src/client/mir_client_library.cpp (+31/-31)
src/client/mir_client_surface.h (+2/-2)
src/client/mir_connection.cpp (+23/-23)
src/client/mir_connection.h (+1/-1)
src/client/mir_surface.cpp (+19/-19)
src/client/mir_surface.h (+1/-1)
src/client/mir_wait_handle.cpp (+5/-5)
src/client/mir_wait_handle.h (+5/-3)
tests/unit-tests/client/gbm/test_mesa_native_display_container.cpp (+1/-1)
tests/unit-tests/client/test_aging_buffer.cpp (+1/-6)
tests/unit-tests/client/test_wait_handle.cpp (+0/-2)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

It is not pointless - it just doesn't change the linkage.

It permits C++ code to refer to use the namespace for disambiguation, and it cleanly separates the API in the generated Docygen docs.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I still don't see the use.

The client identifiers are clearly namespaced (C-style) by the "mir_" prefix already. Hence they're already unambiguous.

It's just a matter of risk. You're solving a problem that will probably never exist. That is someone defines a "mir_connect" elsewhere. And the trade-off is that the code is made uglier for this highly unlikely situation.

Taking it to an extreme, someone could still come along and define:
    somethingelse::mir_toolkit::mir_connect
albeit even less likely than the previous case. We're talking about highly unlikely vs exceedingly unlikely. So I would prefer to err on the side of code cleanliness.

Daniel van Vugt (vanvugt)
Changed in mir:
assignee: Alan Griffiths (alan-griffiths) → nobody
Daniel van Vugt (vanvugt)
information type: Proprietary → Public
Robert Carr (robertcarr)
Changed in mir:
status: New → Fix Committed
Daniel van Vugt (vanvugt)
Changed in mir:
assignee: nobody → Alan Griffiths (alan-griffiths)
Daniel van Vugt (vanvugt)
Changed in mir:
milestone: none → 0.0.3
Robert Ancell (robert-ancell)
Changed in mir:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.