aml-property-server: fix coverity issue [1/1]
PD#SWPL-172617
Problem:
TOCTOU
COPY_INSTEAD_OF_MOVE
Solution:
fix coverity issue
change list api
remove aml-dbus verbose log
Verify:
t5w
Change-Id: I82034d4dd53fb12fe141cde39bd24781c0ef0417
Signed-off-by: Jun Zhang <jun.zhang@amlogic.com>
diff --git a/aml_dbus/aml-dbus.h b/aml_dbus/aml-dbus.h
index a395567..706899a 100644
--- a/aml_dbus/aml-dbus.h
+++ b/aml_dbus/aml-dbus.h
@@ -747,7 +747,8 @@
void *val = alloca(pcall->size);
int r;
sd_bus_message *reply = NULL;
- AMBUS_CHECK(r, pcall->dp_call(pcall, val, m), goto done);
+ if ((r = pcall->dp_call(pcall, val, m)) < 0)
+ goto done;
if (!sd_bus_message_get_expect_reply(m) || pcall->async_reply) {
r = 1;
goto done;
diff --git a/aml_property_server/dbus/property-client.cpp b/aml_property_server/dbus/property-client.cpp
index f8d36a9..8ffcd4f 100644
--- a/aml_property_server/dbus/property-client.cpp
+++ b/aml_property_server/dbus/property-client.cpp
@@ -89,7 +89,7 @@
std::string v;
bool ret = CALL_DBUS(get, key, v);
if (ret) {
- value = v;
+ value = std::move(v);
}
return ret;
}
@@ -98,7 +98,9 @@
return CALL_DBUS(set, key, value);
}
-std::map<std::string, std::string> list() { return CALL_DBUS(list); }
+bool list(std::map<std::string, std::string> &data) {
+ return CALL_DBUS(list, data);
+}
static int dbus_signal_handle(sd_bus_message *message, void *userdata,
sd_bus_error *error) {
diff --git a/aml_property_server/dbus/property-server.cpp b/aml_property_server/dbus/property-server.cpp
index 04ec212..e53cfb5 100644
--- a/aml_property_server/dbus/property-server.cpp
+++ b/aml_property_server/dbus/property-server.cpp
@@ -45,7 +45,9 @@
return Property::set(key, value);
}
-std::map<std::string, std::string> list() { return Property::list(); }
+bool list(std::map<std::string, std::string> &data) {
+ return Property::list(data);
+}
} // namespace internal
template <typename... Args> struct plf_method_call;
@@ -85,7 +87,7 @@
static const sd_bus_vtable prop_vtable[] = {SD_BUS_VTABLE_START(0),
MAKE_VTABLE_ITEM(plf_method_call, get),
MAKE_VTABLE_ITEM(plf_method_call, set),
- MAKE_VTABLE_ITEM(ambus_method_call, list),
+ MAKE_VTABLE_ITEM(plf_method_call, list),
SD_BUS_SIGNAL_WITH_NAMES(PROP_DBUS_SIGNAL, "isssss",
SD_BUS_PARAM(event) SD_BUS_PARAM(namespace)
SD_BUS_PARAM(key) SD_BUS_PARAM(action)
diff --git a/aml_property_server/include/property.h b/aml_property_server/include/property.h
index 89e6126..4139713 100644
--- a/aml_property_server/include/property.h
+++ b/aml_property_server/include/property.h
@@ -56,9 +56,11 @@
/**
* @brief list key/value in the current property database
*
- * @return key/value map
+ * @param data key/value map
+ *
+ * @return true: succeed, false: failed
*/
-std::map<std::string, std::string> list();
+bool list(std::map<std::string, std::string> &data);
struct PropertyChangedEvent {
int event;
diff --git a/aml_property_server/property-db/property-db.cpp b/aml_property_server/property-db/property-db.cpp
index eb0883c..6ec0430 100644
--- a/aml_property_server/property-db/property-db.cpp
+++ b/aml_property_server/property-db/property-db.cpp
@@ -25,6 +25,7 @@
*/
#include <dirent.h>
+#include <fcntl.h>
#include <fstream>
#include <leveldb/db.h>
#include <leveldb/write_batch.h>
@@ -109,17 +110,25 @@
struct dirent *entry;
char full_path[PATH_MAX];
+ int dirfd = open(path, O_DIRECTORY);
+ if (dirfd == -1) {
+ LOGE("open %s failed\n", path);
+ return false;
+ }
+
// Check if path exists
- if (stat(path, &statbuf) == -1) {
- LOGE("stat %s failed\n", path);
+ if (fstatat(dirfd, path, &statbuf, AT_SYMLINK_NOFOLLOW) == -1) {
+ LOGE("fstatat %s failed\n", path);
+ close(dirfd);
return false;
}
// Handle directory
if (S_ISDIR(statbuf.st_mode)) {
- dir = opendir(path);
+ dir = fdopendir(dirfd);
if (dir == NULL) {
LOGE("opendir %s failed\n", path);
+ close(dirfd);
return false;
}
@@ -144,6 +153,8 @@
return true;
}
+ close(dirfd);
+
// Handle file
if (unlink(path) == -1) {
LOGE("unlink %s failed\n", path);
@@ -454,7 +465,7 @@
} else {
e.value = value;
}
- e.oldvalue = old_value;
+ e.oldvalue = std::move(old_value);
notify(e);
}
return ret;
@@ -474,12 +485,11 @@
return internal_set(_key, _value);
}
-map<string, string> list() {
- map<string, string> ret;
+bool list(map<string, string> &data) {
for (int i = 0; i < ATTR_MAX; i++) {
- gs_database[i].list(ret);
+ gs_database[i].list(data);
}
- return ret;
+ return true;
}
bool subscribe(PropertyChangedCallback callback) {
diff --git a/aml_property_server/tools/proputils.cpp b/aml_property_server/tools/proputils.cpp
index 2ea7aec..1a0d5f4 100644
--- a/aml_property_server/tools/proputils.cpp
+++ b/aml_property_server/tools/proputils.cpp
@@ -43,9 +43,11 @@
}
void list_property() {
- auto props = Property::list();
- for (auto &p : props) {
- cout << "[" << p.first << "]: [" << p.second << "]" << endl;
+ std::map<std::string, std::string> props;
+ if (Property::list(props)) {
+ for (auto &p : props) {
+ cout << "[" << p.first << "]: [" << p.second << "]" << endl;
+ }
}
}