Message ID | 20200922133537.258098-2-paul.elder@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Paul, On Tue, Sep 22, 2020 at 10:35:00PM +0900, Paul Elder wrote: > Document the issue related to global variable dependencies and how to > avoid them. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v2 > --- > Documentation/coding-style.rst | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst > index 8af06d6a..967506db 100644 > --- a/Documentation/coding-style.rst > +++ b/Documentation/coding-style.rst > @@ -187,6 +187,21 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide` > long term borrowing isn't marked through language constructs, it shall be > documented explicitly in details in the API. > > +Global Variables > +~~~~~~~~~~~~~~~~ > + > +The order of initialization and destructions of global variables cannot be > +reasonably controlled. This can cause problems (segfaults) when global > +variables depend on each other, or when non-globals depend on globals. > +For example, if the declaration of a global variable calls a constructor, > +which calls into another global variable that has yet to be initialized, that > +is likely to segfault. > + > +The best solution is to avoid global variables when possible. If required, > +such as for implementing factories with auto-registration, avoid dependencies > +by running the minimum amount of code in the constructor and destructor, > +and move code that contains dependencies to a later point of time. > + While avoiding globals is desirable, sometimes it makes things easier, and if the type of the variable is trivially destructible and has a constexpr constructor we should be safe against initialization/destruction order failures. This section is fine as it is a 'stay safe' kind-of paragraph, but if you feel like expanding it you can have some references from here https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables Thanks j > C Compatibility Headers > ~~~~~~~~~~~~~~~~~~~~~~~ > > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Paul, Thank you for the patch. On Wed, Sep 23, 2020 at 12:13:16PM +0200, Jacopo Mondi wrote: > On Tue, Sep 22, 2020 at 10:35:00PM +0900, Paul Elder wrote: > > Document the issue related to global variable dependencies and how to > > avoid them. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > New in v2 > > --- > > Documentation/coding-style.rst | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst > > index 8af06d6a..967506db 100644 > > --- a/Documentation/coding-style.rst > > +++ b/Documentation/coding-style.rst > > @@ -187,6 +187,21 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide` > > long term borrowing isn't marked through language constructs, it shall be > > documented explicitly in details in the API. > > > > +Global Variables > > +~~~~~~~~~~~~~~~~ > > + > > +The order of initialization and destructions of global variables cannot be > > +reasonably controlled. This can cause problems (segfaults) when global s/segfaults/including segfaults/ as there can be other types of problems. > > +variables depend on each other, or when non-globals depend on globals. The latter isn't possible, at least directly, as all global variables will be constructed before anything else happens. I would write "depend on each other, directly or indirectly". > > +For example, if the declaration of a global variable calls a constructor, > > +which calls into another global variable that has yet to be initialized, that "calling into a variable" sounds weird. Maybe "which uses another global variable that hasn't been initialized yet" ? > > +is likely to segfault. As there are other possible issues than segfaults, "[...], incorrect behaviour is likely.". And I would add "Similar issues may occur when the library is unloaded and global variables are destroyed." > > + > > +The best solution is to avoid global variables when possible. If required, Not all of them though, there's no need to avoid global variables that have constexpr constructors and trivial destructors. I would write Global variables that are statically initialized and have trivial destructors (such as an integer constant for instance) do no cause any issue. Other global variables shall be avoided when possible, but are allowed when required (for instance to implement factories with auto-registration). They shall no depend on any other global variable, by running ...". > > +such as for implementing factories with auto-registration, avoid dependencies > > +by running the minimum amount of code in the constructor and destructor, > > +and move code that contains dependencies to a later point of time. > > + > > While avoiding globals is desirable, sometimes it makes things easier, > and if the type of the variable is trivially destructible and has a It should be trivially constructible too :-) Or at least the constructor and destructor should not call into other global objects. > constexpr constructor we should be safe against > initialization/destruction order failures. > > This section is fine as it is a 'stay safe' kind-of paragraph, but if > you feel like expanding it you can have some references from here > https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables Good point. We have less strict rules though. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > C Compatibility Headers > > ~~~~~~~~~~~~~~~~~~~~~~~ > >
diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index 8af06d6a..967506db 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -187,6 +187,21 @@ These rules match the `object ownership rules from the Chromium C++ Style Guide` long term borrowing isn't marked through language constructs, it shall be documented explicitly in details in the API. +Global Variables +~~~~~~~~~~~~~~~~ + +The order of initialization and destructions of global variables cannot be +reasonably controlled. This can cause problems (segfaults) when global +variables depend on each other, or when non-globals depend on globals. +For example, if the declaration of a global variable calls a constructor, +which calls into another global variable that has yet to be initialized, that +is likely to segfault. + +The best solution is to avoid global variables when possible. If required, +such as for implementing factories with auto-registration, avoid dependencies +by running the minimum amount of code in the constructor and destructor, +and move code that contains dependencies to a later point of time. + C Compatibility Headers ~~~~~~~~~~~~~~~~~~~~~~~
Document the issue related to global variable dependencies and how to avoid them. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v2 --- Documentation/coding-style.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+)