| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Part of enforcing cpp core guidelines involves explicitly including all
constructors required on a non-trivial class. We were missing quite a
few. In all cases, the copy/move/and operator= methods are simply
deleted.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
|
|
clang-tidy added cppcoreguidelines-init-variables as a check, which is
something we already enforce to some extent, but getting CI to enforce
it will help reviews move faster.
Tested: Code compiles. Noop changes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7e10950de617b1d3262265572b1703f2e60b69d0
|
|
clang-13 includes new checks, and finds some issues. The first is that
the boost::vector constructor can possibly throw, so replace the
underlying flat_map container with std::vector instead.
The others are places where we could possibly throw in destructors,
which would be bad. Ideally we wouldn't use the destructor pattern, but
that would be non-trivial to clean up at this point, so just catch the
exception, and log it. At the same time, catch exceptions thrown to
main and log them.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I77b86eaa2fc79e43d1ca044c78ca3b0ce0a7c38c
|
|
For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, and objects by const reference.
Tested: Code compiles. Trivial changes.
This saves about 300 bytes on our compressed binary size.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4
|
|
Ideally, we'd use the DbusVariantType for all variant uses within bmcweb
to help with binary size. This commit adds all of the missing types to
DbusVariantType in the pursuit of this goal
Adding these new types made the struct pretty unwieldy, so as part of
that port, it disables clang-format and puts each item on its own line
to help with readability. At some point in the future, this list could
be alphabetized, but the ordering has the potential to change the
function of this, so it's avoided for the moment.
As an unrelated note, it turns out that the dbus-rest API never knew how
to serialize file descriptors. Using a FD off the system doesn't make
much sense, but now that we have a common variant type, we're required
to provide serialization specializations, so for the moment this code
just converts it to an int.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ice1953a163c761024f969acf1aa2654a8a7e9661
|
|
This reverts commit 91995f3272010875e1559397e98ca93354066a0e.
Seeing bumps fail.
https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/48864
Please fix, test, and resubmit.
Change-Id: Id539fe66d5a093caf8f22a393f7af7b58ead5247
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
These modifications are from WIP:Redfish:Query parameters:Only
(https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/47474). And they
will be used in Redfish:Query Parameters:Only.
(https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/38952)
The code changed the completion handle to accept Res to be able to
recall handle with a new Response object.
AsyncResp owns a new res, so there is no need to pass in a res.
Tested:
1.Basic and Token auth both still work.
2.Use scripts/websocket_test.py to test websockets. It is still work
correctly.
python3 websocket_test.py --host 127.0.0.1:2443
This modification is a public part, so you can use any URL to test
this function. The response is the same as before.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I570e32fb47a9a90fe111fcd1f4054060cd21def3
|
|
GET on URI "/bus/system/" causes bmcweb service to crash.
This occurred as asyncResp was captured by reference by lambda.
Tested:
- GET on /bus/system/ responded with desired output without crash.
Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
Change-Id: I2c777be4bc2c95332d47df380c867891b133d016
|
|
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
|
|
This reverts commit b623d9c1b6605978eb6158619bb43c79a9f543fd.
Fixes #206
There are conditions where the mapper apparently replies with errors.
While this is obviously wrong, it causes a user facing regression in
firmware updates, which is documented in #206. This change is likely
just fine in isolation, but someone will need to resolve the mapper
issue before we can re-merge this.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I59843b6cc001a81e68b4f82da922b7eb2611ca29
|
|
Symptom:
Run automation test "Create Two User Initiated Dump And Delete One"
then we got ERROR as below.
Create Two User Initiated Dump And Delete One | FAIL |
Test Bmc Dump :: Test dump functionality of OpenBMC. | FAIL |
**ERROR** 200 != 404
Root cause:
handleGet() function of dbus_rest in bmcweb should be return setErrorResponse()
when doing get request after delete 1 bmc dump.
After checking code flow that will print "message 200 OK",
not "message 404 Not Found".
Due to /xyz/openbmc_project/dump/bmc/entry/1 object path already be deleted and not exist.
Thus when doing get request again with the deleted object path,
we will found that async_send() call return "Bad dbus request error".
But, code flow will keep to response with [200], not [404].
Response should be [404] not [200]. That's why auto test report "**ERROR** 200 != 404".
Solution:
Add setErrorResponse with not found error message return
when async_send return "Bad dbus request error".
Tested:
robot -t Create_Two_User_Initiated_Dump_And_Delete_One redfish/extended/test_bmc_dump.robot
Create Two User Initiated Dump And Delete One | PASS |
Test Bmc Dump :: Test dump functionality of OpenBMC. | PASS |
1 critical test, 1 passed, 0 failed
1 test total, 1 passed, 0 failed
Signed-off-by: Tim Lee <timlee660101@gmail.com>
Change-Id: I81e4f22bc2a24f482a41b757835068ca81321437
|
|
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
|
|
The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.
This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.
Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0
|
|
camelLower is not a type, camelBack is.
Changes were made automatically with clang-tidy --fix-errors
To be able to apply changes automatically, the only way I've found that
works was to build the version of clang/clang-tidy that yocto has, and
run the fix script within bitbake -c devshell bmcweb. Unfortunately,
yocto has clang-tidy 11, which can apparently find a couple extra errors
in tests we already had enabled. As such, a couple of those are also
included.
Tested:
Ran clang-tidy-11 and got a clean result.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9d1080b67f0342229c2f267160849445c065ca51
|
|
1st, alphabetize the tidy-list for good housekeeping.
Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Using std::move on const variables. This does nothing.
2. Passing big variables (like std::string) by value.
3. Using double quotes on a find call, which constructs an intermediate
string, rather than using the character overload.
Tested
Loaded on system, logged in successfully and pulled down webui-vue. No
new errors.
Walked the Redfish tree a bit, and observed no new problems.
Ran redfish service validator. Got no new failures (although there are
a lot of log service deprecation warnings that we should look at).
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I2238958c4b22c1e554e09a0a1787c744bdbca43e
|
|
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
|
|
(In the voice of the kid from sixth sense) I see string copies...
Apparently there are a lot of places we make unnecessary copies. This
fixes all of them.
Not sure how to split this up into smaller patches, or if it even needs
split up. It seems pretty easy to review to me, because basically every
diff is identical.
Change-Id: I22b4ae4f96f7e4082d2bc701098a04f7bed95369
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
|
|
Now that CI can handle clang-tidy, and a lot of the individual fixes
have landed for the various static analysis checks, lets see how close
we are.
This includes bringing a bunch of the code up to par with the checks
that require. Most of them fall into the category of extraneous else
statements, const correctness problems, or extra copies.
Tested:
CI only. Unit tests pass.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9fbd346560a75fdd3901fa40c57932486275e912
|
|
As part of getting the clang-tidy bugprone tests running, these
conditionals were found to have duplicate entries, or be somewhat
nonsensical.
Tested: clang-tidy now passes with the branch-compliance check set.
Change-Id: Ibec106f3bbd866fc471a1fc56bd8cdd5d44024e0
|
|
Lots of code has been checked in that doesn't match the naming
conventions. Lets fix that.
Tested:
Code compiles. Variable/function renames only.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I6bd107811d0b724f1fad990016113cdf035b604b
|
|
There were a couple places in code where we still use index based for
loops. Move these to the more modern range based for loops.
Tested: Needs testing. Changes made by clang-tidy.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I30bf6fae6b2540434d5c98900a8f6bd0c8f2be93
|
|
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>
|
|
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
|
|
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>
|
|
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>
|
|
These spelling errors were found using
https://github.com/codespell-project/codespell
Tested: Built and ran against validator.
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
Change-Id: I214fe102550295578cfdf0fc58305897d261ce55
|
|
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>
|
|
Add missing res.end() to avoid the hang/timeout.
Tested:
Enabled DBUS and unit tested specified code and
observed no hang.
Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
Change-Id: I29457b9cebea4e2699c8227a46d1d5e06578a5bf
|
|
clang-tidy flagged an error where strings were being constructed at
startup. Move them to const char* to save a little memory, and reduce
the possibility of a error being thrown at startup.
Tested: Code compiles. Error codes need tested functionally.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I227e91879e727f4b19d955111b0d2bac8e81b6ad
|
|
This was an automatic change made by clang-tidy. It moves all uses of
NULL to nullptr, which are equivalent, but nullptr is prefered.
Tested: Code compiles.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I9526599b222693c9723a69934b599c7a5b5d1fbf
|
|
We had a couple uses of push_back in the code that could be made more
efficient with emplace(). Use them instead.
Tested: No functional change. Needs tested.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I417601e416b1d0be989617a372978d52670135d8
|
|
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
|
|
Recently, a number of people in the community have made the (admittedly
easy) mistake that we use a significant portion of crow.
Today, we use crow for the router, and the "app" structure, and even
those have been significantly modified to meet the bmc needs. All other
components have been replaced with Boost beast. This commit removes the
crow mentions from the Readme, and moves the crow folder to "http" to
camouflage it a little. No code content has changed.
Tested:
Code compiles. No functional change made to any executable code.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: Iceb57b26306cc8bdcfc77f3874246338864fd118
|
|
-werror on the newest version of GCC finds even more stuff than was
found before. Fix all of them.
Tested: No functional change. In theory these cases can't occur unless
a dbus interface is broken.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: Id11e29e4851075b511e69cbc006aa8d7e1e229f0
|
|
using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.
Tested:
It builds. Will test various subsystems that have been touched
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I588c26440e5a97f718a0f0ea74cc84107d53aa1e
|
|
Add a range check for numerical values so that they are not truncated.
Tested:
<type>:<interface>:<property>
- bool: RebootPolicy: AutoReboot
Valid: 0, 1
Invalid: null, -1, 2
- int64_t: Ambient Temp Sensor: WarningHigh
Valid: -9223372036854775808, -1, 2500, 9223372036854775807
Invalid: null, -9223372036854775809
- uint8_t: Software: Priority
Valid: 0, 1, 255
Invalid: null, -1, 256
- uint16_t: LED Physical: Period
Valid: 0, 1000, 65535
Invalid: null, -1, 65536
- uint32_t: State PowerOnHours: POHCounter
Valid: 0, 20, 4294967295
Invalid: -1, 4294967296
- uint64_t: State BMC: LastRebootTime
Valid: 0, 1566402464000, 18446744073709551615
Invalid: -1, 18446744073709551616
Closes: openbmc/bmcweb#101
Change-Id: I652333b0042b28ffb0a47b478d1a0a6e7ec994a7
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
|
|
Issue:
The downloaded dump file name is having dump id instead of actual dump
file name.
Solution:
Added "Content-Disposition" header into http response packet with
filename as actual dump file name. So, The downloaded dump file
will be saved in actual dump file name when downloading the dump file
by using dump id.
Tested By:
- curl -O -J -c cjar -b cjar -k -H "X-Auth-Token: $bmc_token"
https: //$bmc_ip/download/dump/DUMP_ID
Change-Id: Id4726da20081e7d57d62038f672169f440edecfd
Signed-off-by: Ramesh Iyyar <rameshi1@in.ibm.com>
|
|
When invoking a D-Bus method call via the 'action' URL, return
the error that came back from the D-Bus call as opposed to just
hardcoding one.
Tested: A POST on /xyz/openbmc_project/dump/action/CreateDump when
no more dumps can be created now returns:
{
"data": {
"description": "xyz.openbmc_project.Dump.Create.Error.QuotaExceeded"
},
"message": "Dump not captured due to a cap.",
"status": "error"
}
Change-Id: Ifd0c97f82ff05842fa0f36ef3bb1aaba42ad7d49
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
|
|
1. Move the system endpoints to AsyncResp where possible. This starts
to clean up our scope issues, and makes the code a bit cleaner, as it's
not tabbed in as much. It's by no means a fix, but it certainly is
better, and easier to verify. Also it gives us throw protection as far
as the connection objects go.
2. Implement the "properties" field when accessing urls like:
/bus/system/<serviceName>/<ObjectPath>/<InterfaceName>
Tested:
Called GET on
/bus/system/xyz.openbmc_project.FanSensor/xyz/openbmc_project/sensors/fan_tach/Fan_1/xyz.openbmc_project.Sensor.Value
and observed the response:
{
"bus_name": "xyz.openbmc_project.FanSensor",
"interface": "xyz.openbmc_project.Sensor.Value",
"methods": [],
"object_path": "/xyz/openbmc_project/sensors/fan_tach/Fan_1",
"properties": {
"MaxValue": 14000.0,
"MinValue": 0.0,
"Value": null
},
"signals": [],
"status": "ok"
}
Previous to this patch, properties was an empty object {}
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I8fceb395fb64f2a1857df8ba64b5914c09c18552
|
|
sdbusplus adds message::get_error() to provide the actual sd_bus_error
of a message.
With this, return the error's name and message in REST API so that the
correct error is returned to end user.
Tested: Verify the REST API output when trying to set host time while
the settings does not allow that:
$ curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X PUT -d '{"data": 1436655598435272}' https://$bmc/xyz/openbmc_project/time/bmc/attr/Elapsed
{
"data": {
"description": "xyz.openbmc_project.Time.Error.NotAllowed"
},
"message": "The operation is not allowed",
"status": "error"
}
Resolves openbmc/bmcweb#83
Change-Id: I0bd5405c6748d124f9dd8a341e29f3918445158e
Signed-off-by: Lei YU <mine260309@gmail.com>
|
|
The boost::system::error_code returned by the DBus call will contain the
DBus error name and description in its error_category, use the
category's name and message as REST API's description and message.
Tested: Verify the REST API output when trying to set host time while
the settings does not allow that:
$ curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X PUT -d '{"data": 1436655598435272}' https://$bmc/xyz/openbmc_project/time/bmc/attr/Elapsed
{
"data": {
"description": "xyz.openbmc_project.Time.Error.NotAllowed"
},
"message": "The operation is not allowed",
"status": "error"
}
Resolves openbmc/bmcweb#83
Change-Id: I90c11c0fc61e55329c809ecb5f948ae041a579d0
Signed-off-by: Lei YU <mine260309@gmail.com>
|
|
We're at CPP17 everywhere now, no need to keep the
experimental refrerence.
Tested: It builds
Change-Id: I5f6571eb411bf055e9715f7d96d1be5a3cb2e119
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
A recent change to the URL routing broke all /org endpoints, which now
return 404 instead of the correct data. This resolves it, and points at
the correct object paths again.
Tested:
Given this is a CI failure, will rely on CI to test the /org endpoints.
Resolves #72
Change-Id: I779bb32f1f2bcba45fdb64f5bf510e7fa832e2d2
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
|
|
This patchset is the beginings of the infrastructure to allow
separate registrations, and map privileges to the actual node in the
url table rather than having each registration manage privileges
manually.
Tested by:
Running redfish compliance tool. All things still pass.
Change-Id: I72d278cc19c60ba5b6e563fbd705b0551faf9a6a
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
|
|
This reverts commit 6ea007a2faec52ad62680015d2a3f00371a1e351.
Reason for revert: Reports of bmcweb seg faults.
Change-Id: I408f1bb29c2f8e427a6621cdaac8c31b847ebf06
|
|
bmcweb classically has not taken a strong opinion on warnings. With
this commit, that policy is changing, and bmcweb will invoke the best
warnings we are able to enable, and turn on -Werror for all builds.
This is intended to reduce the likelihood of hard-to-debug situations
that the compiler coulve caught early on.
Change-Id: I57474410821e82666b3a108cfd0db7d070e8900a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
A quick scan with infer, a static analysis package.
https://fbinfer.com/docs/getting-started.html
Revealed a couple of legitimate bugs. I'm attaching the people on the
blame result to this review so they can look over the change. These are
unlikely to be exploitable in practice, but we should fix them anyway,
to clean up the analysis results.
Tested By:
Code still compiles, changes should be no-op.
Change-Id: I615dad6eb86fa2ea1709e2e2b009d07036d5f8de
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
|
|
Change-Id: I9d7069668f91f2ac72d2f4a440f63e0e85dd5269
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
|
|
Use new_method_call() + async_send() to get back an sd_bus_message
from the org.freedesktop.DBus.Properties.GetAll call in the GET
handler, and then use convertDBusToJSON to extract any possible
property type instead of having to use a variant with all possible
property types defined ahead of time.
Tested: Did a get on several different paths, including one in
/org/open_power/ that had a signature of a(tx) that previously
didn't return anything.
Resolves openbmc/bmcweb#34
Change-Id: I40309664fa969741c4af9a60b9059c60bf6f35f4
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
|
|
When convertDBusToJSON processed an sd_bus_message with
a signature that had multiple high level entries, the
handler code tried to reassign a reference variable to
a new array entry, but references can't be reassigned,
so all it did was reset the passed in JSON object to
the empty value at the end of the array.
Instead, do this with a pointer.
Tested: A message with a signature of "sa{sv}as" was failing,
with nlohmann::json complaining about trying to do a
push_back on a string object. With this fix, that no longer
happens.
Change-Id: Idb3d3a56f0bd38f559f96f828ad95db65bbd11e1
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
|
