A question about bmcweb code

[Ed Tanous]

On 4/29/19 10:59 PM, Lei YU wrote:
> This email is to ask a question about the code in bmcweb.
> 
> When I started to review/read some code in bmcweb, some code concerns me.
> 
> 1. There is a static systemBus in `include/dbus_singleton.hpp`, which is
>    included by multiple header files;
>    In case this is included by multiple cpp files, there will be different
>    instances of `systemBus` in different compile units, which makes it not
>    singleton at all
> 2. There are static variables in multiple header files in `includes/`, e.g.
>    `include/obmc_console.hpp`
>    In case these are included by multiple cpp files, we got different instances,
>    and some of the variables are large and thus consume memory.
> 
> Luckily, current bmcweb only includes all the header files
> src/webserver_main.cpp, so it does not have issues mentioned above.
> 
> So my question is, is it a design to enforce all implementation in headers and
> only included by `webserver_main.cpp`?

Today bmcweb is built as a (mostly) single compile unit because of some
history, and some heavy template usage early on that was inherited from
crow.  A lot of that has been cleaned up in the last while.  As you say,
the mediocre singletons don't present a functional issue at this point,
but it's definitely something that could be improved.

My goal long term is to see things moved into appropriate separate
compile units as make sense, with the goal of decreasing the build
times, but I haven't really had time to execute on it as of late.


> If yes, I would expect some description in README or some document;
> If no, how do we avoid the issues described above?

This isn't documented because we aren't consistent about it today, so
documenting it would only really be documenting the statement on where
we want to get to.  Once it's made consistent, we will absolutely
document the rules.

Any patches you want to send in to fix some, or all of these issues
would be greatly appreciated and reviewed.

> 
> Thanks!
>