[Libstoragemgmt-devel] [PATCH] C API: Initialize listing counts to zero

[Tony Asleson]

Make sure we initialize list counts to zero so that in
the case where we fail the count is returned to the user
as zero to prevent errors in user programs if they don't
pass in a zero value for count.

Also removed some duplicated code.

Signed-off-by: Tony Asleson <***@redhat.com>
---
include/libstoragemgmt/libstoragemgmt.h | 2 +-
src/lsm_convert.cpp | 4 ++
src/lsm_convert.hpp | 11 +++++-
src/lsm_mgmt.cpp | 69 ++++++++-------------------------
test/tester.c | 64 ++++++++++++++++--------------
5 files changed, 65 insertions(+), 85 deletions(-)

diff --git a/include/libstoragemgmt/libstoragemgmt.h b/include/libstoragemgmt/libstoragemgmt.h
index b18adff..021dddc 100644
--- a/include/libstoragemgmt/libstoragemgmt.h
+++ b/include/libstoragemgmt/libstoragemgmt.h
@@ -551,7 +551,7 @@ extern "C" {
* Retrieves a list of access groups.
* @param[in] conn Valid connection @see lsm_connect_password
* @param[out] groups Array of access groups
- * @param[out] group_count Size of array
+ * @param[out] group_count Size of array
* @param[in] flags Reserved for future use, must be zero.
* @return LSM_ERR_OK on success, else error reason.
*/
diff --git a/src/lsm_convert.cpp b/src/lsm_convert.cpp
index 9785d5b..44a191b 100644
--- a/src/lsm_convert.cpp
+++ b/src/lsm_convert.cpp
@@ -78,6 +78,8 @@ int value_array_to_volumes(Value &volume_values, lsm_volume **volumes[],
{
int rc = LSM_ERR_OK;
try {
+ *count = 0;
+
if( Value::array_t == volume_values.valueType()) {
std::vector<Value> vol = volume_values.asArray();

@@ -163,6 +165,8 @@ int value_array_to_disks(Value &disk_values, lsm_disk **disks[], uint32_t *count
{
int rc = LSM_ERR_OK;
try {
+ *count = 0;
+
if( Value::array_t == disk_values.valueType()) {
std::vector<Value> d = disk_values.asArray();

diff --git a/src/lsm_convert.hpp b/src/lsm_convert.hpp
index 988fa6c..49d1310 100644
--- a/src/lsm_convert.hpp
+++ b/src/lsm_convert.hpp
@@ -51,6 +51,14 @@ lsm_volume * value_to_volume(Value &vol);
*/
Value volume_to_value(lsm_volume *vol);

+
+/**
+ * Converts a vector of volume values to an array
+ * @param volume_values Vector of values that represents volumes
+ * @param volumes An array of volume pointers
+ * @param count Number of volumes
+ * @return LSM_ERR_OK on success, else error reason
+ */
int value_array_to_volumes(Value &volume_values, lsm_volume **volumes[],
uint32_t *count);

@@ -75,7 +83,8 @@ Value disk_to_value(lsm_disk *disk);
* @param count Number of disks
* @return LSM_ERR_OK on success, else error reason.
*/
-int value_array_to_disks(Value &disk_values, lsm_disk **disks[], uint32_t *count);
+int value_array_to_disks(Value &disk_values, lsm_disk **disks[],
+ uint32_t *count);

/**
* Converts a value to lsm_initiator *
diff --git a/src/lsm_mgmt.cpp b/src/lsm_mgmt.cpp
index 6a87522..8d59137 100644
--- a/src/lsm_mgmt.cpp
+++ b/src/lsm_mgmt.cpp
@@ -671,6 +671,8 @@ static int get_initiator_array(lsm_connect *c, int rc, Value &response,
lsm_initiator **initiators[], uint32_t *count)
{
try {
+ *count = 0;
+
if( LSM_ERR_OK == rc && Value::array_t == response.valueType()) {
std::vector<Value> inits = response.asArray();

@@ -721,36 +723,15 @@ int lsm_initiator_list(lsm_connect *c, lsm_initiator **initiators[],
return get_initiator_array(c, rc, response, initiators, count);
}

-static int get_volume_array(lsm_connect *c, int rc, Value response,
+static int get_volume_array(lsm_connect *c, int rc, Value &response,
lsm_volume **volumes[], uint32_t *count)
{
- try {
- if( LSM_ERR_OK == rc && Value::array_t == response.valueType()) {
- std::vector<Value> vol = response.asArray();
-
- *count = vol.size();
+ if( LSM_ERR_OK == rc && Value::array_t == response.valueType()) {
+ rc = value_array_to_volumes(response, volumes, count);

- if( vol.size() ) {
- *volumes = lsm_volume_record_array_alloc(vol.size());
-
- if( *volumes ){
- for( size_t i = 0; i < vol.size(); ++i ) {
- (*volumes)[i] = value_to_volume(vol[i]);
- }
- } else {
- rc = LSM_ERR_NO_MEMORY;
- }
- }
- }
- } catch( const ValueException &ve) {
- if( *volumes && *count ) {
- lsm_volume_record_array_free(*volumes, *count);
- *volumes = NULL;
- *count = 0;
+ if( LSM_ERR_OK != rc ) {
+ rc = logException(c, LSM_ERR_INTERNAL_ERROR, "Unexpected type", NULL);
}
-
- rc = logException(c, LSM_ERR_INTERNAL_ERROR, "Unexpected type",
- ve.what());
}
return rc;
}
@@ -761,7 +742,8 @@ int lsm_volume_list(lsm_connect *c, lsm_volume **volumes[], uint32_t *count,
{
CONN_SETUP(c);

- if( !volumes || !count || CHECK_RP(volumes) || LSM_FLAG_UNUSED_CHECK(flags) ) {
+ if( !volumes || !count || CHECK_RP(volumes) ||
+ LSM_FLAG_UNUSED_CHECK(flags) ) {
return LSM_ERR_INVALID_ARGUMENT;
}

@@ -778,35 +760,15 @@ int lsm_volume_list(lsm_connect *c, lsm_volume **volumes[], uint32_t *count,
static int get_disk_array(lsm_connect *c, int rc, Value &response,
lsm_disk **disks[], uint32_t *count)
{
- try {
- if( LSM_ERR_OK == rc && Value::array_t == response.valueType()) {
- std::vector<Value> d = response.asArray();
-
- *count = d.size();
-
- if( d.size() ) {
- *disks = lsm_disk_record_array_alloc(d.size());
+ if( LSM_ERR_OK == rc && Value::array_t == response.valueType()) {
+ rc = value_array_to_disks(response, disks, count);

- if( *disks ){
- for( size_t i = 0; i < d.size(); ++i ) {
- (*disks)[i] = value_to_disk(d[i]);
- }
- } else {
- rc = LSM_ERR_NO_MEMORY;
- }
- }
- }
- } catch( const ValueException &ve ) {
- rc = logException(c, LSM_ERR_INTERNAL_ERROR, "Unexpected type",
- ve.what());
- if( *disks && *count ) {
- lsm_disk_record_array_free(*disks, *count);
- *disks = NULL;
- *count = 0;
+ if( LSM_ERR_OK != rc ) {
+ rc = logException(c, LSM_ERR_INTERNAL_ERROR, "Unexpected type", NULL);
}
}
- return rc;

+ return rc;
}

int lsm_disk_list(lsm_connect *c, lsm_disk **disks[],
@@ -1433,7 +1395,8 @@ int lsm_volumes_accessible_by_initiator(lsm_connect *c,
return LSM_ERR_INVALID_INIT;
}

- if( CHECK_RP(volumes) || !count || LSM_FLAG_UNUSED_CHECK(flags) ){
+ if( CHECK_RP(volumes) || !count || *count != 0 ||
+ LSM_FLAG_UNUSED_CHECK(flags) ){
return LSM_ERR_INVALID_ARGUMENT;
}

diff --git a/test/tester.c b/test/tester.c
index 71eb2d1..40ef059 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -521,6 +521,7 @@ START_TEST(test_smoke_test)
}

lsm_initiator **inits = NULL;
+ count = 0;
/* Get a list of initiators */
rc = lsm_initiator_list(c, &inits, &count, LSM_FLAG_RSVD);

@@ -534,6 +535,7 @@ START_TEST(test_smoke_test)
create_volumes(c, selectedPool, 3);

lsm_volume **volumes = NULL;
+ count = 0;
/* Get a list of volumes */
rc = lsm_volume_list(c, &volumes, &count, LSM_FLAG_RSVD);

@@ -551,48 +553,50 @@ START_TEST(test_smoke_test)
lsm_volume_op_status_get(volumes[i]));
}

+ if( count ) {

- lsm_volume *rep = NULL;
- char *job = NULL;
+ lsm_volume *rep = NULL;
+ char *job = NULL;

- //Try a re-size then a snapshot
- lsm_volume *resized = NULL;
- char *resizeJob = NULL;
+ //Try a re-size then a snapshot
+ lsm_volume *resized = NULL;
+ char *resizeJob = NULL;

- int resizeRc = lsm_volume_resize(c, volumes[0],
- ((lsm_volume_number_of_blocks_get(volumes[0]) *
- lsm_volume_block_size_get(volumes[0])) * 2), &resized, &resizeJob, LSM_FLAG_RSVD);
+ int resizeRc = lsm_volume_resize(c, volumes[0],
+ ((lsm_volume_number_of_blocks_get(volumes[0]) *
+ lsm_volume_block_size_get(volumes[0])) * 2), &resized, &resizeJob, LSM_FLAG_RSVD);

- fail_unless(resizeRc == LSM_ERR_OK || resizeRc == LSM_ERR_JOB_STARTED,
- "lsmVolumeResize %d (%s)", resizeRc,
- error(lsm_error_last_get(c)));
+ fail_unless(resizeRc == LSM_ERR_OK || resizeRc == LSM_ERR_JOB_STARTED,
+ "lsmVolumeResize %d (%s)", resizeRc,
+ error(lsm_error_last_get(c)));

- if( LSM_ERR_JOB_STARTED == resizeRc ) {
- resized = wait_for_job_vol(c, &resizeJob);
- }
+ if( LSM_ERR_JOB_STARTED == resizeRc ) {
+ resized = wait_for_job_vol(c, &resizeJob);
+ }

- lsm_volume_record_free(resized);
+ lsm_volume_record_free(resized);

- //Lets create a snapshot of one.
- int repRc = lsm_volume_replicate(c, NULL, //Pool is optional
- LSM_VOLUME_REPLICATE_SNAPSHOT,
- volumes[0], "SNAPSHOT1",
- &rep, &job, LSM_FLAG_RSVD);
+ //Lets create a snapshot of one.
+ int repRc = lsm_volume_replicate(c, NULL, //Pool is optional
+ LSM_VOLUME_REPLICATE_SNAPSHOT,
+ volumes[0], "SNAPSHOT1",
+ &rep, &job, LSM_FLAG_RSVD);

- fail_unless(repRc == LSM_ERR_OK || repRc == LSM_ERR_JOB_STARTED,
- "lsmVolumeReplicate %d (%s)", repRc,
- error(lsm_error_last_get(c)));
+ fail_unless(repRc == LSM_ERR_OK || repRc == LSM_ERR_JOB_STARTED,
+ "lsmVolumeReplicate %d (%s)", repRc,
+ error(lsm_error_last_get(c)));

- if( LSM_ERR_JOB_STARTED == repRc ) {
- rep = wait_for_job_vol(c, &job);
- }
+ if( LSM_ERR_JOB_STARTED == repRc ) {
+ rep = wait_for_job_vol(c, &job);
+ }

- lsm_volume_record_free(rep);
+ lsm_volume_record_free(rep);

- lsm_volume_record_array_free(volumes, count);
+ lsm_volume_record_array_free(volumes, count);

- if (pools) {
- lsm_pool_record_array_free(pools, poolCount);
+ if (pools) {
+ lsm_pool_record_array_free(pools, poolCount);
+ }
}
}