Discussion:
[Libstoragemgmt-devel] [PATCH] C: Remove ambiguity in class checking
Tony Asleson
2014-08-21 22:50:43 UTC
Permalink
The function value_to_* make sure that the value we are converting
too matches the class in question. If for some reason the json
does not match expected we return a NULL, which is the same as if
we had a failure to allocate memory. Added checks before to
ensure that we have the correct json representation of the class
before we convert from Value type to C struct type.

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_convert.cpp | 46 +++++++++++++++++++-------------------
c_binding/lsm_convert.hpp | 35 +++++++++++++++++++++++++++++
c_binding/lsm_plugin_ipc.cpp | 53 ++++++++++++++++++++++----------------------
3 files changed, 85 insertions(+), 49 deletions(-)

diff --git a/c_binding/lsm_convert.cpp b/c_binding/lsm_convert.cpp
index 554d2da..1eb4331 100644
--- a/c_binding/lsm_convert.cpp
+++ b/c_binding/lsm_convert.cpp
@@ -22,7 +22,7 @@
#include "libstoragemgmt/libstoragemgmt_blockrange.h"
#include "libstoragemgmt/libstoragemgmt_nfsexport.h"

-static bool is_expected_object(Value &obj, std::string class_name)
+bool is_expected_object(Value &obj, std::string class_name)
{
if (obj.valueType() == Value::object_t) {
std::map<std::string, Value> i = obj.asObject();
@@ -38,7 +38,7 @@ lsm_volume *value_to_volume(Value &vol)
{
lsm_volume *rc = NULL;

- if (is_expected_object(vol, "Volume")) {
+ if (is_expected_object(vol, CLASS_NAME_VOLUME)) {
std::map<std::string, Value> v = vol.asObject();

rc = lsm_volume_record_alloc(v["id"].asString().c_str(),
@@ -59,7 +59,7 @@ Value volume_to_value(lsm_volume *vol)
{
if( LSM_IS_VOL(vol) ) {
std::map<std::string, Value> v;
- v["class"] = Value("Volume");
+ v["class"] = Value(CLASS_NAME_VOLUME);
v["id"] = Value(vol->id);
v["name"] = Value(vol->name);
v["vpd83"] = Value(vol->vpd83);
@@ -113,7 +113,7 @@ int value_array_to_volumes(Value &volume_values, lsm_volume **volumes[],
lsm_disk *value_to_disk(Value &disk)
{
lsm_disk *rc = NULL;
- if (is_expected_object(disk, "Disk")) {
+ if (is_expected_object(disk, CLASS_NAME_DISK)) {
std::map<std::string, Value> d = disk.asObject();

rc = lsm_disk_record_alloc(d["id"].asString().c_str(),
@@ -133,7 +133,7 @@ Value disk_to_value(lsm_disk *disk)
{
if ( LSM_IS_DISK(disk) ) {
std::map<std::string, Value> d;
- d["class"] = Value("Disk");
+ d["class"] = Value(CLASS_NAME_DISK);
d["id"] = Value(disk->id);
d["name"] = Value(disk->name);
d["disk_type"] = Value(disk->disk_type);
@@ -185,7 +185,7 @@ lsm_pool *value_to_pool(Value &pool)
{
lsm_pool *rc = NULL;

- if (is_expected_object(pool, "Pool")) {
+ if (is_expected_object(pool, CLASS_NAME_POOL)) {
std::map<std::string, Value> i = pool.asObject();

rc = lsm_pool_record_alloc(i["id"].asString().c_str(),
@@ -205,7 +205,7 @@ Value pool_to_value(lsm_pool *pool)
{
if( LSM_IS_POOL(pool) ) {
std::map<std::string, Value> p;
- p["class"] = Value("Pool");
+ p["class"] = Value(CLASS_NAME_POOL);
p["id"] = Value(pool->id);
p["name"] = Value(pool->name);
p["element_type"] = Value(pool->element_type);
@@ -223,7 +223,7 @@ Value pool_to_value(lsm_pool *pool)
lsm_system *value_to_system(Value &system)
{
lsm_system *rc = NULL;
- if (is_expected_object(system, "System")) {
+ if (is_expected_object(system, CLASS_NAME_SYSTEM)) {
std::map<std::string, Value> i = system.asObject();

rc = lsm_system_record_alloc(i["id"].asString().c_str(),
@@ -239,7 +239,7 @@ Value system_to_value(lsm_system *system)
{
if( LSM_IS_SYSTEM(system)) {
std::map<std::string, Value> s;
- s["class"] = Value("System");
+ s["class"] = Value(CLASS_NAME_SYSTEM);
s["id"] = Value(system->id);
s["name"] = Value(system->name);
s["status"] = Value(system->status);
@@ -290,7 +290,7 @@ lsm_access_group *value_to_access_group( Value &group )
lsm_string_list *il = NULL;
lsm_access_group *ag = NULL;

- if( is_expected_object(group, "AccessGroup")) {
+ if( is_expected_object(group, CLASS_NAME_ACCESS_GROUP)) {
std::map<std::string, Value> vAg = group.asObject();
il = value_to_string_list(vAg["init_ids"]);

@@ -312,7 +312,7 @@ Value access_group_to_value( lsm_access_group *group )
{
if( LSM_IS_ACCESS_GROUP(group) ) {
std::map<std::string, Value> ag;
- ag["class"] = Value("AccessGroup");
+ ag["class"] = Value(CLASS_NAME_ACCESS_GROUP);
ag["id"] = Value(group->id);
ag["name"] = Value(group->name);
ag["init_ids"] = Value(string_list_to_value(group->initiators));
@@ -364,7 +364,7 @@ Value access_group_list_to_value( lsm_access_group **group, uint32_t count)
lsm_block_range *value_to_block_range(Value &br)
{
lsm_block_range *rc = NULL;
- if( is_expected_object(br, "BlockRange") ) {
+ if( is_expected_object(br, CLASS_NAME_BLOCK_RANGE) ) {
std::map<std::string, Value> range = br.asObject();

rc = lsm_block_range_record_alloc(range["src_block"].asUint64_t(),
@@ -378,7 +378,7 @@ Value block_range_to_value(lsm_block_range *br)
{
if( LSM_IS_BLOCK_RANGE(br) ) {
std::map<std::string, Value> r;
- r["class"] = Value("BlockRange");
+ r["class"] = Value(CLASS_NAME_BLOCK_RANGE);
r["src_block"] = Value(br->source_start);
r["dest_block"] = Value(br->dest_start);
r["block_count"] = Value(br->block_count);
@@ -423,7 +423,7 @@ Value block_range_list_to_value( lsm_block_range **brl, uint32_t count )
lsm_fs *value_to_fs(Value &fs)
{
lsm_fs *rc = NULL;
- if( is_expected_object(fs, "FileSystem") ) {
+ if( is_expected_object(fs, CLASS_NAME_FILE_SYSTEM) ) {
std::map<std::string, Value> f = fs.asObject();

rc = lsm_fs_record_alloc(f["id"].asString().c_str(),
@@ -441,7 +441,7 @@ Value fs_to_value(lsm_fs *fs)
{
if( LSM_IS_FS(fs) ) {
std::map<std::string, Value> f;
- f["class"] = Value("FileSystem");
+ f["class"] = Value(CLASS_NAME_FILE_SYSTEM);
f["id"] = Value(fs->id);
f["name"] = Value(fs->name);
f["total_space"] = Value(fs->total_space);
@@ -458,7 +458,7 @@ Value fs_to_value(lsm_fs *fs)
lsm_fs_ss *value_to_ss(Value &ss)
{
lsm_fs_ss *rc = NULL;
- if( is_expected_object(ss, "FsSnapshot") ) {
+ if( is_expected_object(ss, CLASS_NAME_FS_SNAPSHOT) ) {
std::map<std::string, Value> f = ss.asObject();

rc = lsm_fs_ss_record_alloc(f["id"].asString().c_str(),
@@ -473,7 +473,7 @@ Value ss_to_value(lsm_fs_ss *ss)
{
if( LSM_IS_SS(ss) ) {
std::map<std::string, Value> f;
- f["class"] = Value("FsSnapshot");
+ f["class"] = Value(CLASS_NAME_FS_SNAPSHOT);
f["id"] = Value(ss->id);
f["name"] = Value(ss->name);
f["ts"] = Value(ss->ts);
@@ -486,7 +486,7 @@ Value ss_to_value(lsm_fs_ss *ss)
lsm_nfs_export *value_to_nfs_export(Value &exp)
{
lsm_nfs_export *rc = NULL;
- if( is_expected_object(exp, "NfsExport") ) {
+ if( is_expected_object(exp, CLASS_NAME_FS_EXPORT) ) {
int ok = 0;
lsm_string_list *root = NULL;
lsm_string_list *rw = NULL;
@@ -539,7 +539,7 @@ Value nfs_export_to_value(lsm_nfs_export *exp)
{
if( LSM_IS_NFS_EXPORT(exp) ) {
std::map<std::string, Value> f;
- f["class"] = Value("NfsExport");
+ f["class"] = Value(CLASS_NAME_FS_EXPORT);
f["id"] = Value(exp->id);
f["fs_id"] = Value(exp->fs_id);
f["export_path"] = Value(exp->export_path);
@@ -560,7 +560,7 @@ Value nfs_export_to_value(lsm_nfs_export *exp)
lsm_storage_capabilities *value_to_capabilities(Value &exp)
{
lsm_storage_capabilities *rc = NULL;
- if( is_expected_object(exp, "Capabilities") ) {
+ if( is_expected_object(exp, CLASS_NAME_CAPABILITIES) ) {
const char *val = exp["cap"].asC_str();
rc = lsm_capability_record_alloc(val);
}
@@ -572,7 +572,7 @@ Value capabilities_to_value(lsm_storage_capabilities *cap)
if( LSM_IS_CAPABILITIY(cap) ) {
std::map<std::string, Value> c;
char *t = capability_string(cap);
- c["class"] = Value("Capabilities");
+ c["class"] = Value(CLASS_NAME_CAPABILITIES);
c["cap"] = Value(t);
free(t);
return Value(c);
@@ -584,7 +584,7 @@ Value capabilities_to_value(lsm_storage_capabilities *cap)
lsm_target_port *value_to_target_port(Value &tp)
{
lsm_target_port *rc = NULL;
- if( is_expected_object(tp, "TargetPort") ) {
+ if( is_expected_object(tp, CLASS_NAME_TARGET_PORT) ) {
rc = lsm_target_port_record_alloc(
tp["id"].asC_str(),
(lsm_target_port_type)tp["port_type"].asInt32_t(),
@@ -602,7 +602,7 @@ Value target_port_to_value(lsm_target_port *tp)
{
if( LSM_IS_TARGET_PORT(tp) ) {
std::map<std::string, Value> p;
- p["class"] = Value("TargetPort");
+ p["class"] = Value(CLASS_NAME_TARGET_PORT);
p["id"] = Value(tp->id);
p["port_type"] = Value(tp->port_type);
p["service_address"] = Value(tp->service_address);
diff --git a/c_binding/lsm_convert.hpp b/c_binding/lsm_convert.hpp
index b0c7586..0c495ae 100644
--- a/c_binding/lsm_convert.hpp
+++ b/c_binding/lsm_convert.hpp
@@ -24,6 +24,41 @@
#include "lsm_ipc.hpp"

/**
+ * Class names for serialized json
+ */
+const char CLASS_NAME_SYSTEM[] = "System";
+const char CLASS_NAME_POOL[] = "Pool";
+const char CLASS_NAME_VOLUME[] = "Volume";
+const char CLASS_NAME_BLOCK_RANGE[] = "BlockRange";
+const char CLASS_NAME_ACCESS_GROUP[] = "AccessGroup";
+const char CLASS_NAME_FILE_SYSTEM[] = "FileSystem";
+const char CLASS_NAME_DISK[] = "Disk";
+const char CLASS_NAME_FS_SNAPSHOT[] = "FsSnapshot";
+const char CLASS_NAME_FS_EXPORT[] = "NfsExport";
+const char CLASS_NAME_CAPABILITIES[] = "Capabilities";
+const char CLASS_NAME_TARGET_PORT[] = "TargetPort";
+
+
+#define IS_CLASS(x, name) is_expected_object(x, name)
+
+#define IS_CLASS_SYSTEM(x) IS_CLASS(x, CLASS_NAME_SYSTEM)
+#define IS_CLASS_POOL(x) IS_CLASS(x, CLASS_NAME_POOL)
+#define IS_CLASS_VOLUME(x) IS_CLASS(x, CLASS_NAME_VOLUME)
+#define IS_CLASS_BLOCK_RANGE(x) IS_CLASS(x, CLASS_NAME_BLOCK_RANGE)
+#define IS_CLASS_ACCESS_GROUP(x) IS_CLASS(x, CLASS_NAME_ACCESS_GROUP)
+#define IS_CLASS_FILE_SYSTEM(x) IS_CLASS(x, CLASS_NAME_FILE_SYSTEM)
+
+
+
+/**
+ * Checks to see if a value is an expected object instance
+ * @param obj Value to check
+ * @param class_name Class name to check
+ * @return boolean, true if matches
+ */
+bool LSM_DLL_LOCAL is_expected_object(Value &obj, std::string class_name);
+
+/**
* Converts an array of Values to a lsm_string_list
* @param list List represented as an vector of strings.
* @return lsm_string_list pointer, NULL on error.
diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index ac1be4b..24b89ae 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -552,7 +552,7 @@ static int capabilities(lsm_plugin_ptr p, Value &params, Value &response)

Value v_s = params["system"];

- if( Value::object_t == v_s.valueType() &&
+ if( IS_CLASS_SYSTEM(v_s) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_system *sys = value_to_system(v_s);

@@ -670,7 +670,7 @@ static int handle_volume_create(lsm_plugin_ptr p, Value &params, Value &response
Value v_size = params["size_bytes"];
Value v_prov = params["provisioning"];

- if( Value::object_t == v_p.valueType() &&
+ if( IS_CLASS_POOL(v_p) &&
Value::string_t == v_name.valueType() &&
Value::numeric_t == v_size.valueType() &&
Value::numeric_t == v_prov.valueType() &&
@@ -713,7 +713,7 @@ static int handle_volume_resize(lsm_plugin_ptr p, Value &params, Value &response
Value v_vol = params["volume"];
Value v_size = params["new_size_bytes"];

- if( Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_VOLUME(v_vol) &&
Value::numeric_t == v_size.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {

@@ -755,8 +755,10 @@ static int handle_volume_replicate(lsm_plugin_ptr p, Value &params, Value &respo
Value v_rep = params["rep_type"];
Value v_name = params["name"];

- if( (Value::object_t == v_pool.valueType() || Value::null_t == v_pool.valueType()) &&
- Value::object_t == v_vol_src.valueType() &&
+ if( ((Value::object_t == v_pool.valueType() &&
+ IS_CLASS_POOL(v_pool)) ||
+ Value::null_t == v_pool.valueType()) &&
+ IS_CLASS_VOLUME(v_vol_src) &&
Value::numeric_t == v_rep.valueType() &&
Value::string_t == v_name.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
@@ -802,7 +804,7 @@ static int handle_volume_replicate_range_block_size( lsm_plugin_ptr p,
if( p && p->san_ops && p->san_ops->vol_rep_range_bs ) {
Value v_s = params["system"];

- if( Value::object_t == v_s.valueType() &&
+ if( IS_CLASS_SYSTEM(v_s) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_system *sys = value_to_system(v_s);

@@ -838,8 +840,8 @@ static int handle_volume_replicate_range(lsm_plugin_ptr p, Value &params,
Value v_ranges = params["ranges"];

if( Value::numeric_t == v_rep.valueType() &&
- Value::object_t == v_vol_src.valueType() &&
- Value::object_t == v_vol_dest.valueType() &&
+ IS_CLASS_VOLUME(v_vol_src) &&
+ IS_CLASS_VOLUME(v_vol_dest) &&
Value::array_t == v_ranges.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {

@@ -883,9 +885,9 @@ static int handle_volume_delete(lsm_plugin_ptr p, Value &params, Value &response
if( p && p->san_ops && p->san_ops->vol_delete ) {
Value v_vol = params["volume"];

- if(Value::object_t == v_vol.valueType() &&
+ if(IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
- lsm_volume *vol = value_to_volume(params["volume"]);
+ lsm_volume *vol = value_to_volume(v_vol);

if( vol ) {
char *job = NULL;
@@ -920,7 +922,7 @@ static int handle_vol_online_offline( lsm_plugin_ptr p, Value &params,

Value v_vol = params["volume"];

- if( Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_volume *vol = value_to_volume(v_vol);
if( vol ) {
@@ -998,7 +1000,7 @@ static int ag_create(lsm_plugin_ptr p, Value &params, Value &response)
if( Value::string_t == v_name.valueType() &&
Value::string_t == v_init_id.valueType() &&
Value::numeric_t == v_init_type.valueType() &&
- Value::object_t == v_system.valueType() &&
+ IS_CLASS_SYSTEM(v_system) &&
LSM_FLAG_EXPECTED_TYPE(params)) {

lsm_access_group *ag = NULL;
@@ -1034,7 +1036,7 @@ static int ag_delete(lsm_plugin_ptr p, Value &params, Value &response)
if( p && p->san_ops && p->san_ops->ag_delete ) {
Value v_access_group = params["access_group"];

- if( Value::object_t == v_access_group.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_access_group) &&
LSM_FLAG_EXPECTED_TYPE(params)) {

lsm_access_group *ag = value_to_access_group(v_access_group);
@@ -1064,7 +1066,7 @@ static int ag_initiator_add(lsm_plugin_ptr p, Value &params, Value &response)
Value v_init_type = params["init_type"];


- if( Value::object_t == v_group.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_group) &&
Value::string_t == v_init_id.valueType() &&
Value::numeric_t == v_init_type.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
@@ -1108,7 +1110,7 @@ static int ag_initiator_del(lsm_plugin_ptr p, Value &params, Value &response)
Value v_init_id = params["init_id"];
Value v_init_type = params["init_type"];

- if( Value::object_t == v_group.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_group) &&
Value::string_t == v_init_id.valueType() &&
Value::numeric_t == v_init_type.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
@@ -1150,8 +1152,8 @@ static int volume_mask(lsm_plugin_ptr p, Value &params, Value &response)
Value v_group = params["access_group"];
Value v_vol = params["volume"];

- if( Value::object_t == v_group.valueType() &&
- Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_group) &&
+ IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {

lsm_access_group *ag = value_to_access_group(v_group);
@@ -1184,8 +1186,8 @@ static int volume_unmask(lsm_plugin_ptr p, Value &params, Value &response)
Value v_group = params["access_group"];
Value v_vol = params["volume"];

- if( Value::object_t == v_group.valueType() &&
- Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_group) &&
+ IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params)) {

lsm_access_group *ag = value_to_access_group(v_group);
@@ -1216,7 +1218,7 @@ static int vol_accessible_by_ag(lsm_plugin_ptr p, Value &params, Value &response
if( p && p->san_ops && p->san_ops->vol_accessible_by_ag ) {
Value v_access_group = params["access_group"];

- if( Value::object_t == v_access_group.valueType() &&
+ if( IS_CLASS_ACCESS_GROUP(v_access_group) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_access_group *ag = value_to_access_group(v_access_group);

@@ -1258,7 +1260,7 @@ static int ag_granted_to_volume(lsm_plugin_ptr p, Value &params, Value &response

Value v_vol = params["volume"];

- if( Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_volume *volume = value_to_volume(v_vol);

@@ -1299,7 +1301,7 @@ static int volume_dependency(lsm_plugin_ptr p, Value &params, Value &response)

Value v_vol = params["volume"];

- if( Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
lsm_volume *volume = value_to_volume(v_vol);

@@ -1334,7 +1336,7 @@ static int volume_dependency_rm(lsm_plugin_ptr p, Value &params, Value &response

Value v_vol = params["volume"];

- if( Value::object_t == v_vol.valueType() &&
+ if( IS_CLASS_VOLUME(v_vol) &&
LSM_FLAG_EXPECTED_TYPE(params)) {
lsm_volume *volume = value_to_volume(v_vol);

@@ -1410,7 +1412,7 @@ static int fs_create(lsm_plugin_ptr p, Value &params, Value &response)
Value v_name = params["name"];
Value v_size = params["size_bytes"];

- if( Value::object_t == v_pool.valueType() &&
+ if( IS_CLASS_POOL(v_pool) &&
Value::string_t == v_name.valueType() &&
Value::numeric_t == v_size.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {
@@ -1460,8 +1462,7 @@ static int fs_delete(lsm_plugin_ptr p, Value &params, Value &response)

Value v_fs = params["fs"];

- if( Value::object_t == v_fs.valueType() &&
- LSM_FLAG_EXPECTED_TYPE(params)) {
+ if( IS_CLASS_FILE_SYSTEM(v_fs) && LSM_FLAG_EXPECTED_TYPE(params)) {

lsm_fs *fs = value_to_fs(v_fs);
--
1.8.2.1
Loading...