#
8e8245db |
| 12-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Fix nullptr failures for image upload
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 enable
Fix nullptr failures for image upload
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>
show more ...
|
#
5a39f77a |
| 20-Oct-2023 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository.
Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
show more ...
|
#
80f79a40 |
| 24-Aug-2023 |
Michael Shen <gpgpgp@google.com> |
Fix typo `DBusInteracesMap` -> `DBusInterfacesMap`
Change-Id: I9a851076eccee9d79ad7bb036e58b717e06ad5d1 Signed-off-by: Michael Shen <gpgpgp@google.com>
|
#
3544d2a7 |
| 06-Aug-2023 |
Ed Tanous <edtanous@google.com> |
Use ranges
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to:
``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 's
Use ranges
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to:
``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place ```
Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
62598e31 |
| 17-Jul-2023 |
Ed Tanous <ed@tanous.net> |
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away.
The old style logging (stream based) looked like.
BMCWEB_LOG_DEBUG << "Foo " << foo;
The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo);
In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction.
Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist.
One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it.
Tested: compiled with logging meson_option enabled, and launched bmcweb
Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
show more ...
|
#
2c6ffdb0 |
| 28-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Use openssl random number generator
We already have a generator class. We should use it. Wrap this into a function that can be unit tested, and add unit tests.
Note, some files also needed to cha
Use openssl random number generator
We already have a generator class. We should use it. Wrap this into a function that can be unit tested, and add unit tests.
Note, some files also needed to change name, because random.hpp conflicts with the built in random, and causes circular build problems. This commit changes it to ossl_random.
Tested: Unit tests pass. Now has coverage.
Redfish service validator passes.
Change-Id: I5f8eee1af5f4843a352c6fd0e26d67fd3320ef53 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
33c6b580 |
| 14-Feb-2023 |
Ed Tanous <edtanous@google.com> |
Remove body member from Request
Per cpp core guidelines, these should be methods.
Tested: on last patchset of the series.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib16479db9d2b68d
Remove body member from Request
Per cpp core guidelines, these should be methods.
Tested: on last patchset of the series.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib16479db9d2b68da68e7ad6e825c7e205c64f1de
show more ...
|
#
3ccb3adb |
| 13-Jan-2023 |
Ed Tanous <edtanous@google.com> |
Fix a boatload of #includes
Most of these missing includes were found by running clang-tidy on all files, including headers. The existing scripts just run clang-tidy on source files, which doesn't
Fix a boatload of #includes
Most of these missing includes were found by running clang-tidy on all files, including headers. The existing scripts just run clang-tidy on source files, which doesn't catch most of these.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic741fbb2cc9e5e92955fd5a1b778a482830e80e8
show more ...
|
#
cf9e417d |
| 21-Dec-2022 |
Ed Tanous <edtanous@google.com> |
Add check for globals
We don't follow this cpp core guidelines rule well. This is something that we should aspire to cleaning up in the future, but for the moment, lets turn the rule on in clang-ti
Add check for globals
We don't follow this cpp core guidelines rule well. This is something that we should aspire to cleaning up in the future, but for the moment, lets turn the rule on in clang-tidy to stop the bleeding, add ignores for the things that we know need some better abstractions, and work on these over time.
Most of this commit is just adding NOLINTNEXTLINE exceptions for all of our globals. There was one case in the sensor code where clang correctly noted that those globals weren't actually const, which got missed because of the use of auto.
Tested: CI should be good enough for this. Passes clang-tidy.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ieda08fee69a3b209d4b3e9771809a6c41524f066
show more ...
|
#
59d494ee |
| 22-Jul-2022 |
Patrick Williams <patrick@stwcx.xyz> |
sdbusplus: use shorter type aliases
The sdbusplus headers provide shortened aliases for many types. Switch to using them to provide better code clarity and shorter lines. Possible replacements are
sdbusplus: use shorter type aliases
The sdbusplus headers provide shortened aliases for many types. Switch to using them to provide better code clarity and shorter lines. Possible replacements are for: * bus_t * exception_t * manager_t * match_t * message_t * object_t * slot_t
Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I46a5eec210002af84239af74a93c830b1d4a13f1
show more ...
|
#
002d39b4 |
| 31-May-2022 |
Ed Tanous <edtanous@google.com> |
Try to fix the lambda formatting issue
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels
Try to fix the lambda formatting issue
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels and to use OuterScope."
bmcweb is very callback heavy code. Try to enable it and see if that improves things. There are many cases where the length of a lambda call will change, and reindent the entire lambda function. This is really bad for code reviews, as it's difficult to see the lines changed. This commit should resolve it. This does have the downside of reindenting a lot of functions, which is unfortunate, but probably worth it in the long run.
All changes except for the .clang-format file were made by the robot.
Tested: Code compiles, whitespace changes only.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
show more ...
|
#
1476687d |
| 15-Mar-2022 |
Ed Tanous <edtanous@google.com> |
Remove brace initialization of json objects
Brace initialization of json objects, while quite interesting from an academic sense, are very difficult for people to grok, and lead to inconsistencies.
Remove brace initialization of json objects
Brace initialization of json objects, while quite interesting from an academic sense, are very difficult for people to grok, and lead to inconsistencies. This patchset aims to remove a majority of them in lieu of operator[]. Interestingly, this saves about 1% of the binary size of bmcweb.
This also has an added benefit that as a design pattern, we're never constructing a new object, then moving it into place, we're always adding to the existing object, which in the future _could_ make things like OEM schemas or properties easier, as there's no case where we're completely replacing the response object.
Tested: Ran redfish service validator. No new failures.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe
show more ...
|
#
b9d36b47 |
| 26-Feb-2022 |
Ed Tanous <edtanous@google.com> |
Consitently use dbus::utility types
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9546227a19c691b1aecb
Consitently use dbus::utility types
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
show more ...
|
#
168e20c1 |
| 13-Dec-2021 |
Ed Tanous <edtanous@google.com> |
Move to common variant This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall
Move to common variant This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall size. Note, there were a few places where we broke const-correctness in the form of pulling a non-const reference out of a const variant. This new variant now requires const correctness, so some consts are added where required. Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2
show more ...
|
#
432a890c |
| 14-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Remove ambiguous privileges constructor There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple set
Remove ambiguous privileges constructor There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple sets ORed together. To handle this, there were two overloads of the privileges() method, one that took a vector of Privileges, and one that took an initializer_list of const char*. Unfortunately, this leads some code in AccountService to pick the wrong overload when it's called like this .privileges( {{"ConfigureUsers"}, {"ConfigureManager"}, {"ConfigureSelf"}}) This is supposed to be "User must have ConfigureUsers, or ConfigureManager, or ConfigureSelf". Currently, because it selects the wrong overload, it computes to "User must have ConfigureUsers AND ConfigureManager AND ConfigureSelf. The double braces are supposed to cause this to form a vector of Privileges, but it appears that the initializer list gets consumed, and the single invocation of initializer list is called. Interestingly, trying to put in a privileges overload of intializer_list<initializer_list<const char*>> causes the compilation to fail with an ambiguous call error, which is what I would've expected to see previously in this case, but alas, I'm only a novice when it comes to how the C++ standard works in these edge cases. This is likely due in part to the fact that they were templates of an unused template param (seemingly copied from the previous method) and SFINAE rules around templates. This commit functionally removes one of the privileges overloads, and adds a second set of braces to every privileges call that previously had a single set of braces. Previous code will not compile now, which is IMO a good thing. This likely popped up in the Node class removal, because the Node class explicitly constructs a vector of Privilege objects, ensuing it can hit the right overload Tested: Ran Redfish service validator Tested the specific use case outlined on discord with: Creating a new user with operator privilege: ``` redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2 AccountService adduser foo mysuperPass1 Operator ``` Then attempting to list accounts: ``` curl -vvvv --insecure --user foo:mysuperPass1 https://192.168.7.2/redfish/v1/AccountService/Accounts/foo ``` Which succeeded and returned the account in question. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495
show more ...
|
#
8d1b46d7 |
| 31-Mar-2021 |
zhanghch05 <zhanghch05@inspur.com> |
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response getting ended after the first tree is done populating. Don't use res.end() anymore. Tested: 1. Validator passed. Signed-off-by: zhanghaicheng <zhanghch05@inspur.com> Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8
show more ...
|
#
2dfd18ef |
| 17-Dec-2020 |
Ed Tanous <ed@tanous.net> |
Start using sdbusplus::message::filename() Lots of code gets checked in that does this path checking incorrectly. So much so, that we have it documented in COMMON_ERRORS.md, yet, we
Start using sdbusplus::message::filename() Lots of code gets checked in that does this path checking incorrectly. So much so, that we have it documented in COMMON_ERRORS.md, yet, we persist. This patchset starts using the new object_path::filename() method that was added recently to sdbusplus. Overall, it deletes code, and makes for a much better developer experience. Tested: Pulled down several endpoints and verified that filename() method works properly, and the collections are returned as expected. curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/AccountService/Accounts Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ief1e0584394fb139678d3453265f7011bc931f3c
show more ...
|
#
04e438cb |
| 03-Oct-2020 |
Ed Tanous <ed@tanous.net> |
fix include names cppcheck isn't smart enough to recognize these are c++ headers, not c headers. Considering we're already inconsistent about our naming, it's easier to just be cons
fix include names cppcheck isn't smart enough to recognize these are c++ headers, not c headers. Considering we're already inconsistent about our naming, it's easier to just be consistent, and move the last few files to use .hpp instead of .h. Tested: Code builds, no changes. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40
show more ...
|
#
23e64207 |
| 15-Sep-2020 |
Ed Tanous <ed@tanous.net> |
Fix using namespace We inherited a "using namespace" crow. Lets fix it. Tested: Code compiles. No functional changes. Signed-off-by: Ed Tanous <ed@tanous.net> Cha
Fix using namespace We inherited a "using namespace" crow. Lets fix it. Tested: Code compiles. No functional changes. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Id47446150dfb312c5cd84a4b4284fb824eba8021
show more ...
|
#
cb13a392 |
| 25-Jul-2020 |
Ed Tanous <ed@tanous.net> |
Enable unused variable warnings and resolve This commit enables the "unused variables" warning in clang. Throughout this, it did point out several issues that would've been functional
Enable unused variable warnings and resolve This commit enables the "unused variables" warning in clang. Throughout this, it did point out several issues that would've been functional bugs, so I think it was worthwhile. It also cleaned up several unused variable from old constructs that no longer exist. Tested: Built with clang. Code no longer emits warnings. Downloaded bmcweb to system and pulled up the webui, observed webui loads and logs in properly. Change-Id: I51505f4222cc147d6f2b87b14d7e2ac4a74cafa8 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
23a21a1c |
| 24-Jul-2020 |
Ed Tanous <ed@tanous.net> |
Enable clang warnings This commit enables clang warnings, and fixes all warnings that were found. Most of these fall into a couple categories: Variable shadow issues were fixed
Enable clang warnings This commit enables clang warnings, and fixes all warnings that were found. Most of these fall into a couple categories: Variable shadow issues were fixed by renaming variables unused parameter warnings were resolved by either checking error codes that had been ignored, or removing the name of the variable from the scope. Other various warnings were fixed in the best way I was able to come up with. Note, the redfish Node class is especially insidious, as it causes all imlementers to have variables for parameters, regardless of whether or not they are used. Deprecating the Node class is on my list of things to do, as it adds extra overhead, and in general isn't a useful abstraction. For now, I have simply fixed all the handlers. Tested: Added the current meta-clang meta layer into bblayers.conf, and added TOOLCHAIN_pn-bmcweb = "clang" to my local.conf Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ia75b94010359170159c703e535d1c1af182fe700
show more ...
|
#
52cc112d |
| 18-Jul-2020 |
Ed Tanous <ed@tanous.net> |
Remove middlewares Middlewares, while kinda cool from an academic standpoint, make our build times even worse than they already are. Given that we only really use 1 real middleware
Remove middlewares Middlewares, while kinda cool from an academic standpoint, make our build times even worse than they already are. Given that we only really use 1 real middleware today (token auth) and it needs to move into the parser mode anyway (for security limiting buffer sizes), we might as well use this as an opportunity to delete some code. Some other things that happen: 1. Persistent data now moves out of the crow namespace 2. App is no longer a template 3. All request_routes implementations no longer become templates. This should be a decent (unmeasured) win on compile times. This commit was part of a commit previously called "various cleanups". This separates ONLY the middleware deletion part of that. Note, this also deletes about 400 lines of hard to understand code. Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
b41187fa |
| 24-Oct-2019 |
Ed Tanous <ed.tanous@intel.com> |
Deprecate the "" operator, and isEqP While a cool example of how to do string matching in constexpr space, the set of verbs available to HTTP has been fixed for a very long time.
Deprecate the "" operator, and isEqP While a cool example of how to do string matching in constexpr space, the set of verbs available to HTTP has been fixed for a very long time. This was ported over to beast a while back, but we kept the API for.... mediocre reasons of backward compatibility. Remove that, and delete the now unused code. Tested: Built and loaded on a Witherspoon. Validator passes. Signed-off-by: Ed Tanous <ed.tanous@intel.com> Change-Id: Iaf048e196f9b6e71983189877203bf80390df286 Signed-off-by: James Feist <james.feist@linux.intel.com> Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
show more ...
|
#
1214b7e7 |
| 04-Jun-2020 |
Gunnar Mills <gmills@us.ibm.com> |
clang-format: update to latest from docs repo This is from openbmc/docs/style/cpp/.clang-format Other OpenBMC repos are doing the same. Tested: Built and validator passed.
clang-format: update to latest from docs repo This is from openbmc/docs/style/cpp/.clang-format Other OpenBMC repos are doing the same. Tested: Built and validator passed. Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
show more ...
|
#
8251ffee |
| 10-Oct-2019 |
Ed Tanous <ed.tanous@intel.com> |
Add "requires" handlers to all non-trivial routes This commit is the result of an audit to add user levels to the various components that need them. As written: KVM requires ad
Add "requires" handlers to all non-trivial routes This commit is the result of an audit to add user levels to the various components that need them. As written: KVM requires admin privilege Virtual media requires admin privilege image upload requires admin privilege /subscribe API requies Login privilege Signed-off-by: Ed Tanous <ed.tanous@intel.com> Change-Id: I6384f23769a5ac23f653519656721da7373f088f
show more ...
|