Age | Commit message (Collapse) | Author | Files | Lines |
|
Boost process v2 brings some significant benefits to our launching of
processes[1]. In bmcweb terms:
1. The code is radically simpler, which decreaeses compile times, and
reduces the scope for code scanning tools.
2. The code now uses standard asio pipes instead of inventing its own.
3. Separate compilation.
Tested:
We don't have a lot of unit tests for the virtual media stuff that I can
run, but we do have unit tests for credentials pipe, which in this
change have been ported over, so the feature works. Unit tests are
passing.
[1] https://www.boost.org/doc/libs/1_80_0/doc/html/boost_process/v2.html#boost_process.v2.introduction
Change-Id: Ia20226819d75ff6e492f8852185f0b73e8f5cf83
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Currently, the Cookie auth only checks the first cookie header in a
request. This works fine for most things, because a lot of
implementations (browsers) seem to either put the Cookie headers in
alphabetical order, or put them in the order in which they were stored
which in the case of bmcweb, is also alphabetical.
Well, http2 blows this up, because cookies could potentially be in any
order, given the hpack compression techniques, so there's no promise
that Cookie[0] is the Session cookie.
This commit reworks the authentication code to call beasts "equal_range"
getter, which returns the range of all headers that matched. This
allows us to attempt to parse the cookies in whatever order they might
have been received.
The auth routine only tries to log in the first cookie matching
SESSION=, and do not try to handle duplicates, as this might allow
attackers to negate the anti brute force measures by testing multiple
passwords at once
Tested:
With http2 enabled, the UI can now log in more consistently, and in
addition, the HTML redfish pages function more consistently when using
cookie auth.
Redfish service validator passes.
Change-Id: I3a61a5a654f62096ff19cfbfaf0a10f30a1a3605
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
OpenBMC CI has migrated from using `pycodestyle` to `flake8` as part of
this[1] commit. Unlike `pycodestyle` , `flake8` does not rely on the
presence of setup.cfg file in their root path as a trigger, but it runs
on all repositories by default. Hence there is no need of having
setup.cfg file, so removing it from the repository.
[1]: https://github.com/openbmc/openbmc-build-scripts/commit/c5ad7ff440cfd94fc025efbd45a3859475b18820
Change-Id: I3f38138d0cbf96a3248e2baf2ed59a816a043b68
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
|
|
This commit adds HTTPBasicAuth Get/Patch support
Tested By:
Redfish service validator passes.
```
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HTTPBasicAuth":"Enabled"}' https://192.168.7.2/redfish/v1/AccountService
```
Succeeds with various values.
Enabled: Basic auth succeeds.
Disabled: Basic auth no longer works. AccountService reports "Disabled"
For HTTPBasicAuth status.
Change-Id: Ic417bf3cd4135f05ab34c8613c7fbce953157b03
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
As part of the previous patch tests, UpdateService shows this bug, where
on a multipart parser failure, the dbus match object gets instantiated,
and eventually fails. This leads to mediocre logging, and possibly
could leave update service in an undesirable state.
Fix the error by moving the conditional up.
Tested:
Filling a 16MB file with all zeros and sending it now no longer logs
that a monitor has been set up, and returns immediately instead of
waiting for timeout.
```
dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt
curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":[]} ;type=application/json' -H "Expect:" --user "root:0penBmc" -F UpdateFile=@16mb.txt -v
```
Change-Id: I0962d15c624936b4fa199a675123702003dd697b
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
When pushing multi-part payloads, it's quite helpful if the server
supports the header field of "Expect: 100-Continue". What this does, is
on a large file push, allows the server to possibly reject a request
before the payload is actually sent, thereby saving bandwidth, and
giving the user more information.
Bmcweb, since commit 3909dc82a003893812f598434d6c4558107afa28 by James
(merged July 2020) has simply closed the connection if a user attempts
to send too much data, thereby making the bmcweb implementation simpler.
Unfortunately, to a security tester, this has the appearance on the
network as a crash, which will likely then get filed as a "verify this
isn't failing" bug.
In addition, the default args on curl multipart upload enable the
Expect: 100-Continue behavior, so folks testing must've just been
disabling that behavior.
Bmcweb should just support the right thing here. Unfortunately, closing
a connection uncleanly is easy. Closing a connection cleanly is
difficult. This requires a pretty large refactor of the http connection
class to accomplish.
Tested:
Create files of various size and try to send them (Note, default body
limit is 30 MB) and upload them with an without a username.
```
dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt
curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' -F UpdateFile=@32mb.txt -v
```
No Username:
32MB returns < HTTP/1.1 413 Payload Too Large
16MB returns < HTTP/1.1 401 Unauthorized
With Username
32MB returns < HTTP/1.1 413 Payload Too Large
16MB returns < HTTP/1.1 400 Bad Request
Note, in all cases except the last one, the payload is never sent from
curl.
Redfish protocol validator fails no new tests (SSE failure still
present).
Redfish service validator passes.
Change-Id: I72bc8bbc49a05555c31dc7209292f846ec411d43
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Another clang, another list of checks we can enable.
Tested: Clang-tidy passes.
Change-Id: Ib3fcd8046ce9f2caf9f9d95571ffa3cc2f56c596
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In the below code, we move out of Response, then use it to set
unauthorized, which never gets returned to the user. This results in
the browser showing an empty 200 ok request, because while the request
was propagated rejected, the 401 error code didn't get propagated to the
user.
Tested: If not logged in on a chrome browser:
/redfish/v1 -> Returns the UI
/refish/v1/AccountService -> returns a forward to the webui login page.
If logged into the webui.
/redfish/v1/AccountService now returns the expected HTML redfish
representation of the json response.
Change-Id: I2c906f818367ebb253b3e6097e6787ba4c215e0a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This feature was created for a time before webpack had a built in proxy,
and to debug the UI required setting specific flags. The webpack proxy
solves this problem in a much better way, by proxying everything.
This commit is one piece in the solving a use after free bug. Removing
this allows us to no longer have to cache the origin header [1], which
is only used in this mode.
Tested: Code compiles.
[1] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/70850
Change-Id: I01d67006e217c0c9fd2db7526c0ec34b0da068f3
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This TODO has been in bmcweb for a very long time. Implement it.
W3 sets rules for what security policies apply to which content
types[1]. Reading through this, essentially CSP should only apply to
HTML files.
Tested: Unit tests pass. Webui loads properly. Chrome network window
Shows headers show up as expected.
[1] https://www.w3.org/TR/CSP2/#which-policy-applies
Change-Id: I5467d0373832668763c72a66da2a8872e07bfb58
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and
aims to optimize the case where we're writing small payloads (one or two
bytes.) which needs to be optimized in SSL.
bmcweb never writes one or two bytes, we almost always write the full
payload of what we received, so there's no reason to take the binary
size overhead, and additional boost headers that this implementation
requires.
Tested:
This drops the on-target binary size by 2.6%
Redfish service validator passes.
Change-Id: Ie1ae6f197f8e5ed70cf4abc6be9b1b382c42d64d
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Most of these were found by breaking every redfish class handler into
its own compile unit:
When that's done, these missing headers become compile errors. We
should just fix them.
In addition, this allows us to enable automatic header checking in
clang-tidy using misc-header-cleaner. Because the compiler can now
"see" all the defines, it no longer tries to remove headers that it
thinks are unused.
[1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac
Tested: Code compiles.
Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
They seem to have gotten rid of the nghttp2_static target, so go back to
relying on the "normal" target.
Change-Id: Ic44d9ffe5fa2d88f38c018756738197371b0dc89
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This feature was introduced to manage the operation sync at BMC while
multiple clients manage the BMC.
This feature scope has gone away and it is not a simple code to maintain
as per the growing standards of bmcweb.
This commit removes the feature from this repo.
Tested by: Locks routes are not available anymore
Change-Id: I257225cfb1f43d7d5dadb21a28a2ee5345c5112a
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This macro came originally from CROW_ENABLE_SSL, and was used as a macro
to optionally compile without openssl being required.
OpenSSL has been pulled into many other dependencies, and has been
functionally required to be included for a long time, so there's no
reason to hold onto this macro.
Remove most uses of the macro, and for the couple functional places the
macro is used, transition to a constexpr if to enable the TLS paths.
This allows a large simplification of code in some places.
Tested: Redfish service validator passes.
Change-Id: Iebd46a68e5e417b6031479e24be3c21bef782f4c
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Currently `update_schemas.py` generates a schema list definition like
redfish-core/include/schemas.hpp:
```
constexpr std::array schemas {
"AccountService",
"ActionInfo",
...
"OpenBMCAccountService",
};
```
However, if the number of schemas is more than the clang's default
max size, CI may fail.
The default is `-fbracket-depth=256`.
```
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/functional:65:
[1m/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/array:288:52: [0m [0;1;31mfatal error:
instantiating fold expression with 276 arguments exceeded expression nesting limit of 256
288 | -> array<enable_if_t<(is_same_v<_Tp, _Up> && ...), _Tp>, [0m
| [0;1;32m ~~~~~~~~~~~~~~~~~~~~~~~~^~~~
[0m [1m../redfish-core/include/schemas.hpp:17:26: [0m [0;1;30mnote:
while substituting deduced template arguments into function template '<deduction guide for array>' [with _Tp =
const char *, _Up = <const char *, const char *, const char *, const char *, const char *,
...
const char *>] [0m
17 | constexpr std::array schemas { [0m
| [0;1;32m ^
[0m1 error generated.
```
To avoid the failure, we can set the size explicitly like
```
constexpr std::array<std::string_view,277> schemas {
"AccountService",
...
```
Tested:
1) Remove `include_list` so that all possible schemas are to be used
2) Run with the fixed `scripts/update_schemas.py`
3) Compiles successfully
Change-Id: Ib68db9fd3be1b6dbe0c4b5cc0e9a4324966d759e
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
Meson has a style guide, we should follow it. This includes:
4 space indents on new scopes.
Trailing commas on the final entry in a list/array
Tested: Whitespace only. Code compiles.
Change-Id: Ib7f96a2bd722b55410818c766c0261f5d44cb84d
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This function in the next patch will be used for more than just TFTP, so
rename it to match intent, and refactor to use non-TFTP specific types.
Tested: Rename only. Need help on TFTP setups if we need it.
Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This refactor of code is in preparation for adding new SimpleUpdate
types. Separating out TFTP helps to keep code organized.
Tested: Need help here. TFTP isn't enabled a lot.
Change-Id: Ifbdd4b73bb0f9c31092d729d1ec3d3f395f680b8
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Similar to other refactorings we've been doing, make UpdateService call
into methods instead of inline lambdas.
Tested: Redfish service validator passes. Structural changes only.
Change-Id: I96b6db5e14fa0f7d357fb0faf63d0457b7963581
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
There's currently a problem with phosphor-timesyncd, where enabling NTP
doesn't immediately reflect in the system status on return[1]. To say
it another way, NTP is not enabled/disabled atomically, which leads to
the following problem.
// Disable NTP
PATCH /redfish/v1/Managers/bmc/NetworkProtocol
{"NTP":{"ProtocolEnabled": false}}
// Set the time manually
PATCH /redfish/v1/Managers/bmc {"DateTime": "<timestring"}
Doing this in rapid succession leads to a 500 error, which is obviously
a bug. In the prior commit, this error was changed to a
PropertyValueConflict error, which is still incorrect, but at least
informative of what's going on. REST APIs are intended to have CRUD
compliance. The response should not be returned until the value has
been accepted, and not doing that can lead to problems.
This commit changes the backend to use systemd directly, rather than
routing through phosphor-settings, to avoid this race.
Quite possibly resolves #264 but haven't tested that.
Tested: The above procedure succeeds.
[1] https://github.com/systemd/systemd/pull/11424
Change-Id: I19241e7677d9b6415aff79ac65c474ae71984417
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
If we include OpenSSL in extern "C" blocks consistently, c++ warnings no
longer appear. This means we can remove the special case from meson.
Tested: Code compiles when built locally on an ubuntu 22.04 system.
Change-Id: I5add4113b32cd88b7fdd874174c845425a7c287a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
ReadJsonObject isn't required for cases where we don't have a list of
structures, and ideally we should consolidate all fixed readJson calls
in one place (and not have multi-depth readJson calls).
This commit moves all the calls up, and consolidates all the LDAP patch
params into a single struct that can be moved between the layers, rather
than having the parameters individually.
Tested: Does LDAP work anymore? Could use some help if anyone has test
scripts, otherwise code compiles and this is inspection only, but
similar to other mechanical changes we've made recently
Change-Id: I77c0a8b97d4783fdca875c86d7dace122a0a55d7
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
NTPServers is our last usage of nlohmann::json in a readJson unpack.
The capability and unit tests are left in place for that type in case we
need them in the future, but for now, document them as deprecated.
Tested: Redfish service validator passes. Redfish protocol validator
passes most tests (1 known failure in SSE is unrelated to this change).
Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change 1 line in scripts/parse_registries.py and rerun the script.
Long term OpenBMC/bmcweb need more direction here on the Privilege
Registry, but for now continue with the current direction of using the
Privilege Registry and taking it from the DMTF.
There is new entries in 1.4/1.5 that are needed for future development.
Tested: It builds.
Change-Id: I4337dc44e794c58f00f7307ea0508b84f14e8c8f
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
After splitting up the compile units, warnings start showing up on the
console about privileges. This is due to them being a static global,
and not shared between the compile units.
Move the array to a constexpr structure so that all compile units see
the same init.
Change-Id: I6e035342a5b229a0601c02092ca3ce665b14bbdb
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit changes sdbusplus setProperty calls (in various files) to
setDbusProperty method in Redfish namespace that handles all DBus
errors in a consistent manner.
It also handles and translates additional DBus errors to Redfish
specific errors in dbus_utils file.
Tested By:
Not tested yet
Change-Id: If440774879413754f4c24f9b6572c3c9fa1fd033
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
|
|
Beast 331 added the message_generator class, which allows deduplicating
some templated code for the HTTP parser. When we use it, we can drop
our binary size, and ensure that we have code reuse.
This saves 2.2% on the compressed binary size.
Tested: Redfish service validator passes.
Change-Id: I5540d52dc256adfb62507c67ea642a9ea86d27ee
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Several places that call *req.ioService were missing nullptr checks.
Add them, and fix the one case where it might not be filled in.
Tested: With HTTP2 enabled, the following command succeeds.
```
curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:"
```
Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
If we use string_view with std::from_chars, we can use begin() and end()
directly (because they return pointers) and not have to do silly things
like dereference end(), which, while works in practice, is technically
undefined behavior, and some static analyzers complain about it.
Tested: Unit tests pass against both old parsePostCode and new.
Change-Id: Icfdec3b81f4a9c9bed3599571a8bc8779f9bfb98
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Tested: Code compiles.
Change-Id: Ifb254b703b6193a1faf68a54ad823dc807f92514
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change-Id: Ie1a03bac54183b206bf27e37f1fed804601c8643
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Capturing by auto here causes a copy. Found using static analysis.
Change-Id: Ifbb08f9af0cd6eeec1e611c610e7adf53e17665c
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change-Id: If511f1210cca7bd1da3a8c5152688487d3036e2f
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This code used std::remove, which is a mechanism for removing characters
from strings. Clearly it meant std::filesystem::remove(), which removes
files from the filesystem.
Correct it.
Change-Id: I030966203c1682a11c723c596accdf34637dd1ba
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This code path is subtle, but given that slotPresent is only set to true
if totalCores is incremented, there's no way to actually hit this
section of code.
Looking for input on if this is the right behavior.
Change-Id: Ie6dadd2c7a0ca6b8402148ddd9b8a369a4a38b2e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Stage::T is never set, so it can never happen. Remove it entirely.
Found using static analysis.
Tested: Unit tests pass, good coverage here
Change-Id: I0dfb1aad5bef3ab4451df5e81794e56074f6e939
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Static analysis tools complain that for certain template types, these
comparisons do nothing. Doing nothing is what they're intended to do,
as we have other checks.
Add some constexpr if hints so static analysis tools don't complain.
Change-Id: I60297d094626936d021382ccdc11e4c8b698e3d8
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
boost::asio::buffer_copy returns an integer of the number of values
copied. Some static analysis tools mark that value as nodiscard,
although it should never fail. Audit all uses of buffer_copy, and make
sure that they're using the return value. In theory this should have
no change on the behavior.
Change-Id: I6af39b5347954c2932cf3d4e48e96ff9ae01583a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Despite these objects being called "view" they are still relatively
large, as clang-tidy correctly flags, and we ignore.
Change all function uses to capture by:
const boost::urls::url_view_base&
Which is the base class of all boost URL types, and any class (url,
url_view, etc) is convertible to that base.
Change-Id: I8ee2ea3f4cfba38331303a7e4eb520a2b6f8ba92
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Now that we only support string types in the router we no longer need to
build a "Tag" to be used for constructing argument types. Now, we can
just track the number of arguments, which simplifies the code
significantly, and removes the need to convert to and from the tag to
parameter counts.
This in turn deletes a lot of code in the router, removing the need for
tracking tag types.
Tested: Redfish service validator passes. Unit tests pass.
Change-Id: Ide1d665dc1984552681e8c05952b38073d5e32dd
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This code accidentally makes a copy, given that getJsonObject returns a
std::optional<nlohmann::json::object_t> which is then loaded into a
std::optional<nlohmann::json>. Because nlohmann::json is implicitly
constructible from an object_t, this code works and compiles, but we
shouldn't need the intermediate object at all.
Change the code to simply load the value as object_t.
Tested: Unit tests pass. Good coverage.
Change-Id: Ic57953e66958e69a1233e18a5bbd980405cac58e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Before Redfish put all messages in Base, but as the Messages became more
specific Redfish started creating new registries. Redfish might have
went a little registry happy. HeartbeatEvent just has 1 message and all
these new ones registries each just have a handful of messages.
Add the remaining 15 registries:
composition, environmental, ethernetfabric, fabric, heartbeat_event,
job_event, license, logservice, networkdevice, platform, power,
sensor_event, storage_device, telemetry, update.
Some of these are wanted for both current development and future
development but it is hard to decide which ones so just added them all.
power, fabric, telemetry, update are all things we support today. Having
a UpdateInProgress or UpdateSuccessful makes a lot of sense and this
enables that.
Put these alphabetically. Use a new for loop to do this.
Make changes to scripts/parse_registries.py and run the tool.
No difference in size.
Before: 66928640 Apr 10 13:32
obmc-phosphor-image-p10bmc-20240410183051.ext4.mmc.tar
After: 66928640 Apr 10 13:18
obmc-phosphor-image-p10bmc-20240410181439.ext4.mmc.tar
Tested: bmcweb builds.
"./scripts/parse_registries.py --registries license,update" works.
Change-Id: I43b4d041531cf338e9e7e621714ca7d95f6b01a5
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
bmcs might not have the correct time, so allow certificates for 100
years starting from epoch. As is, the script makes the certificate
valid for now + 10 years. After changes make the script valid from
epoch (1970) to 100 years later (2070).
This makes the script run to completion against a qemu instance of the
bmc.
Additional changes include detecting if a CA key is already present, to
not rewrite it. This allows installing a CA certificate on test
machines once, and using it to authenticate forever.
Additionally, add "alternative names" support, for pointing to a bmc at
localhost, or on the default qemu port, which allows these things to
work by default in those scenarios.
Lastly, change the directory to use a path relative to the script path,
instead of relative to current path when generating certificates. This
ensures that certs are always generated in the same place, which helps
when a CA is reused.
Tested: Script runs to completion without errors.
Change-Id: Ia5c31041dd5cb193b897bf1f7bae3cd9767656d0
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Static analysis notes that the values in these functions are never
initialized, and that a small section of the branch is no longer
possible to hit, now that a default case has been added in
4da0490bc07a458ad3fc7d586c7eabf6053c572f
Remove the dead code and initialize variables where appropriate.
Tested: Unit tests pass. Decent coverage here.
Change-Id: I42ec4678672fea5b21f98aaae05dfca0629652e7
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Session might not be initialized, and might be nullptr.
This line was accessing the session BEFORE the nullptr check. Move it
to after. Found using static analysis.
Change-Id: I966c642aee7c76a29c7d0d57d3b78f5f7bef7d62
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Meson supports unity builds[1] natively. There's no reason to continue
with the pseudo unity build we've been using by putting implementations
in header files.
This commit is the first in a long series of starting to break this up
into smaller compile units, in the hopes of dropping incremental compile
times for developers, and reduce the total per-core memory usage that
gcc requires.
This commit breaks out the run() function from main() and the
constructor of RedfishService from redfish.hpp into their own compile
units. According to tracing, even after broken out, these are still by
far the two longest to compile units in the build.
Tested: Code compiles. Debug build on a 24 core build server results in
a decrease in compile time for compiling just bmcweb from 1m38s to
1m22s.
[1] https://mesonbuild.com/Unity-builds.html
Change-Id: Ibf352e8aba61d64c9a41a7a76e94ab3b5a0dde4b
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit will migrate all the setProperty calls initiated by a
redfish"Action" to "setDbusProperty" method in Redfish namespace that
handles all DBuserrors in a consistent manner.
This method will determine if a setProperty is called during redfish
"Action" or just setting of a dbus property and internally call
appropriate methods that handles different set of errors.
All the Redfish action specific errors are defined in error_messages.hpp
file.
This specific change moves setProperty call in hypervisor_system.hpp and
covers errors in the mentioned file only.
Tested-By:
<Yet to test this usecase>
Change-Id: I3da48fbeabcdcf088c4481021232f08a44797c86
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
The logger changes to move to std::format incidentally caused format
errors to no longer be flagged at compile time.
The example here[1] shows that the latest gcc/c++ gave us a way to solve
this, using std::format_string.
This incidentally shows two places where we got the format arguments
wrong, so fix them.
[1] https://stackoverflow.com/questions/72795189/how-can-i-wrap-stdformat-with-my-own-template-function
Change-Id: Id884200e2c98eeaf5ef8db6c1d6362ede2ffb858
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.
Unfortunately, the lambda syntax of
callback{std::forward<Callback>(callback)}
in a capture confuses it, so change usages to
callback = std::forward<Callback>(callback)
to be consistent.
Tested: Redfish service validator passes.
Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384
Signed-off-by: Ed Tanous <ed@tanous.net>
|