shaba/lvm2
1
0
Fork 0
mirror of git://sourceware.org/git/lvm2.git synced 2024-12-04 08:17:12 +03:00
Code Releases Activity

Replace const usage of dm_config_find_node with more appropriate value-lookup

Browse Source 
functionality. A number of bugs (copied and pasted all over the code) should
disappear:

- most string lookup based on dm_config_find_node would segfault when
  encountering a non-zero integer (the intention there was to print an
  error message instead)
- check for required sections in metadata would have been satisfied by
  values as well (i.e. not sections)
- encountering a section in place of expected flag value would have
  segfaulted (due to assumed but unchecked cn->v != NULL)
This commit is contained in:
Petr Rockai 2011-08-31 15:19:19 +00:00
parent c0de52fd2d
commit 97a4b5165e
8 changed files with 101 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
lib/config/config.c
View File

@ -222,7 +222,7 @@ static void _merge_section(struct dm_config_node *cn1, struct dm_config_node *cn
/* Ignore - we don't have any of these yet */
continue;
/* Not already present? */
if (!(oldn = (struct dm_config_node*)dm_config_find_node(cn1->child, cn->key))) {
if (!(oldn = dm_config_find_node(cn1->child, cn->key))) {
_insert_config_node(&cn1->child, cn);
continue;
}
@ -266,7 +266,7 @@ static int _match_host_tags(struct dm_list *tags, const struct dm_config_node *t
int merge_config_tree(struct cmd_context *cmd, struct dm_config_tree *cft,
struct dm_config_tree *newdata)
{
const struct dm_config_node *root = cft->root;
struct dm_config_node *root = cft->root;
struct dm_config_node *cn, *nextn, *oldn, *cn2;
const struct dm_config_node *tn;
@ -280,7 +280,7 @@ int merge_config_tree(struct cmd_context *cmd, struct dm_config_tree *cft,
if (!_match_host_tags(&cmd->tags, tn))
continue;
}
if (!(oldn = (struct dm_config_node *)dm_config_find_node(root, cn->key))) {
if (!(oldn = dm_config_find_node(root, cn->key))) {
_insert_config_node(&cft->root, cn);
/* Remove any "tags" nodes */
for (cn2 = cn->child; cn2; cn2 = cn2->sib) {

98
lib/format_text/import_vsn1.c
View File

@ -108,20 +108,14 @@ static int _is_converting(struct logical_volume *lv)
static int _read_id(struct id *id, const struct dm_config_node *cn, const char *path)
{
const struct dm_config_value *cv;
const char *uuid;
if (!(cn = dm_config_find_node(cn, path))) {
if (!dm_config_get_str(cn, path, &uuid)) {
log_error("Couldn't find uuid.");
return 0;
}
cv = cn->v;
if (!cv || !cv->v.str) {
log_error("uuid must be a string.");
return 0;
}
if (!id_read_format(id, cv->v.str)) {
if (!id_read_format(id, uuid)) {
log_error("Invalid uuid.");
return 0;
}
@ -131,21 +125,21 @@ static int _read_id(struct id *id, const struct dm_config_node *cn, const char *
static int _read_flag_config(const struct dm_config_node *n, uint64_t *status, int type)
{
const struct dm_config_node *cn;
const struct dm_config_value *cv;
*status = 0;
if (!(cn = dm_config_find_node(n, "status"))) {
if (!dm_config_get_list(n, "status", &cv)) {
log_error("Could not find status flags.");
return 0;
}
if (!(read_flags(status, type | STATUS_FLAG, cn->v))) {
if (!(read_flags(status, type | STATUS_FLAG, cv))) {
log_error("Could not read status flags.");
return 0;
}
if ((cn = dm_config_find_node(n, "flags"))) {
if (!(read_flags(status, type, cn->v))) {
if (dm_config_get_list(n, "flags", &cv)) {
if (!(read_flags(status, type, cv))) {
log_error("Could not read flags.");
return 0;
}
@ -164,7 +158,7 @@ static int _read_pv(struct format_instance *fid, struct dm_pool *mem,
{
struct physical_volume *pv;
struct pv_list *pvl;
const struct dm_config_node *cn;
const struct dm_config_value *cv;
uint64_t size;
if (!(pvl = dm_pool_zalloc(mem, sizeof(*pvl))) ||
@ -238,8 +232,8 @@ static int _read_pv(struct format_instance *fid, struct dm_pool *mem,
dm_list_init(&pv->segments);
/* Optional tags */
if ((cn = dm_config_find_node(pvn, "tags")) &&
!(read_tags(mem, &pv->tags, cn->v))) {
if (dm_config_get_list(pvn, "tags", &cv) &&
!(read_tags(mem, &pv->tags, cv))) {
log_error("Couldn't read tags for physical volume %s in %s.",
pv_dev_name(pv), vg->name);
return 0;
@ -297,7 +291,7 @@ static int _read_segment(struct dm_pool *mem, struct volume_group *vg,
{
uint32_t area_count = 0u;
struct lv_segment *seg;
const struct dm_config_node *cn, *sn_child = sn->child;
const struct dm_config_node *sn_child = sn->child;
const struct dm_config_value *cv;
uint32_t start_extent, extent_count;
struct segment_type *segtype;
@ -322,13 +316,9 @@ static int _read_segment(struct dm_pool *mem, struct volume_group *vg,
segtype_str = "striped";
if ((cn = dm_config_find_node(sn_child, "type"))) {
cv = cn->v;
if (!cv || !cv->v.str) {
log_error("Segment type must be a string.");
return 0;
}
segtype_str = cv->v.str;
if (!dm_config_get_str(sn_child, "type", &segtype_str)) {
log_error("Segment type must be a string.");
return 0;
}
if (!(segtype = get_segtype_from_string(vg->cmd, segtype_str)))
@ -350,8 +340,8 @@ static int _read_segment(struct dm_pool *mem, struct volume_group *vg,
return_0;
/* Optional tags */
if ((cn = dm_config_find_node(sn_child, "tags")) &&
!(read_tags(mem, &seg->tags, cn->v))) {
if (dm_config_get_list(sn_child, "tags", &cv) &&
!(read_tags(mem, &seg->tags, cv))) {
log_error("Couldn't read tags for a segment of %s/%s.",
vg->name, lv->name);
return 0;
@ -378,11 +368,10 @@ static int _read_segment(struct dm_pool *mem, struct volume_group *vg,
}
int text_import_areas(struct lv_segment *seg, const struct dm_config_node *sn,
const struct dm_config_node *cn, struct dm_hash_table *pv_hash,
const struct dm_config_value *cv, struct dm_hash_table *pv_hash,
uint64_t status)
{
unsigned int s;
const struct dm_config_value *cv;
struct logical_volume *lv1;
struct physical_volume *pv;
const char *seg_name = dm_config_parent_name(sn);
@ -392,7 +381,7 @@ int text_import_areas(struct lv_segment *seg, const struct dm_config_node *sn,
return 0;
}
for (cv = cn->v, s = 0; cv && s < seg->area_count; s++, cv = cv->next) {
for (s = 0; cv && s < seg->area_count; s++, cv = cv->next) {
/* first we read the pv */
if (cv->type != DM_CFG_STRING) {
@ -503,7 +492,8 @@ static int _read_lvnames(struct format_instance *fid __attribute__((unused)),
unsigned report_missing_devices __attribute__((unused)))
{
struct logical_volume *lv;
const struct dm_config_node *cn;
const char *lv_alloc;
const struct dm_config_value *cv;
if (!(lv = alloc_lv(mem)))
return_0;
@ -523,16 +513,10 @@ static int _read_lvnames(struct format_instance *fid __attribute__((unused)),
}
lv->alloc = ALLOC_INHERIT;
if ((cn = dm_config_find_node(lvn, "allocation_policy"))) {
const struct dm_config_value *cv = cn->v;
if (!cv || !cv->v.str) {
log_error("allocation_policy must be a string.");
return 0;
}
lv->alloc = get_alloc_from_string(cv->v.str);
if (dm_config_get_str(lvn, "allocation_policy", &lv_alloc)) {
lv->alloc = get_alloc_from_string(lv_alloc);
if (lv->alloc == ALLOC_INVALID) {
log_warn("WARNING: Ignoring unrecognised allocation policy %s for LV %s", cv->v.str, lv->name);
log_warn("WARNING: Ignoring unrecognised allocation policy %s for LV %s", lv_alloc, lv->name);
lv->alloc = ALLOC_INHERIT;
}
}
@ -554,8 +538,8 @@ static int _read_lvnames(struct format_instance *fid __attribute__((unused)),
}
/* Optional tags */
if ((cn = dm_config_find_node(lvn, "tags")) &&
!(read_tags(mem, &lv->tags, cn->v))) {
if (dm_config_get_list(lvn, "tags", &cv) &&
!(read_tags(mem, &lv->tags, cv))) {
log_error("Couldn't read tags for logical volume %s/%s.",
vg->name, lv->name);
return 0;
@ -633,7 +617,7 @@ static int _read_sections(struct format_instance *fid,
/* Only report missing devices when doing a scan */
unsigned report_missing_devices = scan_done_once ? !*scan_done_once : 1;
if (!(n = dm_config_find_node(vgn, section))) {
if (!dm_config_get_section(vgn, section, &n)) {
if (!optional) {
log_error("Couldn't find section '%s'.", section);
return 0;
@ -655,7 +639,9 @@ static struct volume_group *_read_vg(struct format_instance *fid,
const struct dm_config_tree *cft,
unsigned use_cached_pvs)
{
const struct dm_config_node *vgn, *cn;
const struct dm_config_node *vgn;
const struct dm_config_value *cv;
const char *str;
struct volume_group *vg;
struct dm_hash_table *pv_hash = NULL, *lv_hash = NULL;
unsigned scan_done_once = use_cached_pvs;
@ -677,12 +663,8 @@ static struct volume_group *_read_vg(struct format_instance *fid,
vgn = vgn->child;
if ((cn = dm_config_find_node(vgn, "system_id")) && cn->v) {
if (!cn->v->v.str) {
log_error("system_id must be a string");
goto bad;
}
strncpy(vg->system_id, cn->v->v.str, NAME_LEN);
if (dm_config_get_str(vgn, "system_id", &str)) {
strncpy(vg->system_id, str, NAME_LEN);
}
if (!_read_id(&vg->id, vgn, "id")) {
@ -725,16 +707,10 @@ static struct volume_group *_read_vg(struct format_instance *fid,
goto bad;
}
if ((cn = dm_config_find_node(vgn, "allocation_policy"))) {
const struct dm_config_value *cv = cn->v;
if (!cv || !cv->v.str) {
log_error("allocation_policy must be a string.");
goto bad;
}
vg->alloc = get_alloc_from_string(cv->v.str);
if (dm_config_get_str(vgn, "allocation_policy", &str)) {
vg->alloc = get_alloc_from_string(str);
if (vg->alloc == ALLOC_INVALID) {
log_warn("WARNING: Ignoring unrecognised allocation policy %s for VG %s", cv->v.str, vg->name);
log_warn("WARNING: Ignoring unrecognised allocation policy %s for VG %s", str, vg->name);
vg->alloc = ALLOC_NORMAL;
}
}
@ -760,8 +736,8 @@ static struct volume_group *_read_vg(struct format_instance *fid,
}
/* Optional tags */
if ((cn = dm_config_find_node(vgn, "tags")) &&
!(read_tags(vg->vgmem, &vg->tags, cn->v))) {
if (dm_config_get_list(vgn, "tags", &cv) &&
!(read_tags(vg->vgmem, &vg->tags, cv))) {
log_error("Couldn't read tags for volume group %s.", vg->name);
goto bad;
}

2
lib/format_text/text_import.h
View File

@ -20,7 +20,7 @@ struct lv_segment;
struct dm_config_node;
int text_import_areas(struct lv_segment *seg, const struct dm_config_node *sn,
const struct dm_config_node *cn, struct dm_hash_table *pv_hash,
const struct dm_config_value *cv, struct dm_hash_table *pv_hash,
uint64_t status);
#endif

17
lib/mirror/mirrored.c
View File

@ -83,10 +83,10 @@ static int _mirrored_text_import_area_count(const struct dm_config_node *sn, uin
static int _mirrored_text_import(struct lv_segment *seg, const struct dm_config_node *sn,
struct dm_hash_table *pv_hash)
{
const struct dm_config_node *cn;
const struct dm_config_value *cv;
const char *logname = NULL;
if (dm_config_find_node(sn, "extents_moved")) {
if (dm_config_has_node(sn, "extents_moved")) {
if (dm_config_get_uint32(sn, "extents_moved",
&seg->extents_copied))
seg->status |= PVMOVE;
@ -98,7 +98,7 @@ static int _mirrored_text_import(struct lv_segment *seg, const struct dm_config_
}
}
if (dm_config_find_node(sn, "region_size")) {
if (dm_config_has_node(sn, "region_size")) {
if (!dm_config_get_uint32(sn, "region_size",
&seg->region_size)) {
log_error("Couldn't read 'region_size' for "
@ -108,12 +108,7 @@ static int _mirrored_text_import(struct lv_segment *seg, const struct dm_config_
}
}
if ((cn = dm_config_find_node(sn, "mirror_log"))) {
if (!cn->v || !cn->v->v.str) {
log_error("Mirror log type must be a string.");
return 0;
}
logname = cn->v->v.str;
if (dm_config_get_str(sn, "mirror_log", &logname)) {
if (!(seg->log_lv = find_lv(seg->lv->vg, logname))) {
log_error("Unrecognised mirror log in "
"segment %s of logical volume %s.",
@ -130,14 +125,14 @@ static int _mirrored_text_import(struct lv_segment *seg, const struct dm_config_
return 0;
}
if (!(cn = dm_config_find_node(sn, "mirrors"))) {
if (!dm_config_get_list(sn, "mirrors", &cv)) {
log_error("Couldn't find mirrors array for "
"segment %s of logical volume %s.",
dm_config_parent_name(sn), seg->lv->name);
return 0;
}
return text_import_areas(seg, sn, cn, pv_hash, MIRROR_IMAGE);
return text_import_areas(seg, sn, cv, pv_hash, MIRROR_IMAGE);
}
static int _mirrored_text_export(const struct lv_segment *seg, struct formatter *f)

15
lib/raid/raid.c
View File

@ -45,10 +45,9 @@ static int _raid_text_import_area_count(const struct dm_config_node *sn,
static int _raid_text_import_areas(struct lv_segment *seg,
const struct dm_config_node *sn,
const struct dm_config_node *cn)
const struct dm_config_value *cv)
{
unsigned int s;
const struct dm_config_value *cv;
struct logical_volume *lv1;
const char *seg_name = dm_config_parent_name(sn);
@ -57,7 +56,7 @@ static int _raid_text_import_areas(struct lv_segment *seg,
return 0;
}
for (cv = cn->v, s = 0; cv && s < seg->area_count; s++, cv = cv->next) {
for (s = 0; cv && s < seg->area_count; s++, cv = cv->next) {
if (cv->type != DM_CFG_STRING) {
log_error("Bad volume name in areas array for segment %s.", seg_name);
return 0;
@ -104,9 +103,9 @@ static int _raid_text_import(struct lv_segment *seg,
const struct dm_config_node *sn,
struct dm_hash_table *pv_hash)
{
const struct dm_config_node *cn;
const struct dm_config_value *cv;
if (dm_config_find_node(sn, "region_size")) {
if (dm_config_has_node(sn, "region_size")) {
if (!dm_config_get_uint32(sn, "region_size", &seg->region_size)) {
log_error("Couldn't read 'region_size' for "
"segment %s of logical volume %s.",
@ -114,7 +113,7 @@ static int _raid_text_import(struct lv_segment *seg,
return 0;
}
}
if (dm_config_find_node(sn, "stripe_size")) {
if (dm_config_has_node(sn, "stripe_size")) {
if (!dm_config_get_uint32(sn, "stripe_size", &seg->stripe_size)) {
log_error("Couldn't read 'stripe_size' for "
"segment %s of logical volume %s.",
@ -122,14 +121,14 @@ static int _raid_text_import(struct lv_segment *seg,
return 0;
}
}
if (!(cn = dm_config_find_node(sn, "raids"))) {
if (!dm_config_get_list(sn, "raids", &cv)) {
log_error("Couldn't find RAID array for "
"segment %s of logical volume %s.",
dm_config_parent_name(sn), seg->lv->name);
return 0;
}
if (!_raid_text_import_areas(seg, sn, cn)) {
if (!_raid_text_import_areas(seg, sn, cv)) {
log_error("Failed to import RAID images");
return 0;
}

6
lib/striped/striped.c
View File

@ -71,7 +71,7 @@ static int _striped_text_import_area_count(const struct dm_config_node *sn, uint
static int _striped_text_import(struct lv_segment *seg, const struct dm_config_node *sn,
struct dm_hash_table *pv_hash)
{
const struct dm_config_node *cn;
const struct dm_config_value *cv;
if ((seg->area_count != 1) &&
!dm_config_get_uint32(sn, "stripe_size", &seg->stripe_size)) {
@ -80,7 +80,7 @@ static int _striped_text_import(struct lv_segment *seg, const struct dm_config_n
return 0;
}
if (!(cn = dm_config_find_node(sn, "stripes"))) {
if (!dm_config_get_list(sn, "stripes", &cv)) {
log_error("Couldn't find stripes array for segment %s "
"of logical volume %s.", dm_config_parent_name(sn), seg->lv->name);
return 0;
@ -88,7 +88,7 @@ static int _striped_text_import(struct lv_segment *seg, const struct dm_config_n
seg->area_len /= seg->area_count;
return text_import_areas(seg, sn, cn, pv_hash, 0);
return text_import_areas(seg, sn, cv, pv_hash, 0);
}
static int _striped_text_export(const struct lv_segment *seg, struct formatter *f)

7
libdm/libdevmapper.h
View File

@ -1275,7 +1275,8 @@ int dm_config_write_node(const struct dm_config_node *cn, dm_putline_fn putline,
time_t dm_config_timestamp(struct dm_config_tree *cft);
int dm_config_changed(struct dm_config_tree *cft);
struct dm_config_node *dm_config_find_node(const struct dm_config_node *cn, const char *path);
struct dm_config_node *dm_config_find_node(struct dm_config_node *cn, const char *path);
int dm_config_has_node(const struct dm_config_node *cn, const char *path);
const char *dm_config_find_str(const struct dm_config_node *cn, const char *path, const char *fail);
int dm_config_find_int(const struct dm_config_node *cn, const char *path, int fail);
float dm_config_find_float(const struct dm_config_node *cn, const char *path, float fail);
@ -1307,6 +1308,10 @@ int dm_config_get_uint64(const struct dm_config_node *cn, const char *path,
int dm_config_get_str(const struct dm_config_node *cn, const char *path,
const char **result);
int dm_config_get_list(const struct dm_config_node *cn, const char *path,
const struct dm_config_value **result);
int dm_config_get_section(const struct dm_config_node *cn, const char *path,
const struct dm_config_node **result);
unsigned dm_config_maybe_section(const char *str, unsigned len);

43
libdm/libdm-config.c
View File

@ -973,7 +973,7 @@ static int _find_config_bool(const void *start, _node_lookup_fn find,
* node-based lookup
**/
struct dm_config_node *dm_config_find_node(const struct dm_config_node *cn,
struct dm_config_node *dm_config_find_node(struct dm_config_node *cn,
const char *path)
{
return _find_config_node(cn, path);
@ -1038,11 +1038,11 @@ int dm_config_tree_find_bool(const struct dm_config_tree *cft, const char *path,
int dm_config_get_uint32(const struct dm_config_node *cn, const char *path,
uint32_t *result)
uint32_t *result)
{
const struct dm_config_node *n;
n = dm_config_find_node(cn, path);
n = _find_config_node(cn, path);
if (!n || !n->v || n->v->type != DM_CFG_INT)
return 0;
@ -1056,7 +1056,7 @@ int dm_config_get_uint64(const struct dm_config_node *cn, const char *path,
{
const struct dm_config_node *n;
n = dm_config_find_node(cn, path);
n = _find_config_node(cn, path);
if (!n || !n->v || n->v->type != DM_CFG_INT)
return 0;
@ -1070,7 +1070,7 @@ int dm_config_get_str(const struct dm_config_node *cn, const char *path,
{
const struct dm_config_node *n;
n = dm_config_find_node(cn, path);
n = _find_config_node(cn, path);
if (!n || !n->v || n->v->type != DM_CFG_STRING)
return 0;
@ -1079,6 +1079,39 @@ int dm_config_get_str(const struct dm_config_node *cn, const char *path,
return 1;
}
int dm_config_get_list(const struct dm_config_node *cn, const char *path,
const struct dm_config_value **result)
{
const struct dm_config_node *n;
n = _find_config_node(cn, path);
/* TODO when we represent single-item lists consistently, add a check
* for n->v->next != NULL */
if (!n || !n->v)
return 0;
*result = n->v;
return 1;
}
int dm_config_get_section(const struct dm_config_node *cn, const char *path,
const struct dm_config_node **result)
{
const struct dm_config_node *n;
n = _find_config_node(cn, path);
if (!n || n->v)
return 0;
*result = n;
return 1;
}
int dm_config_has_node(const struct dm_config_node *cn, const char *path)
{
return _find_config_node(cn, path) ? 1 : 0;
}
/*
* Convert a token type to the char it represents.
*/