libmount: improve parsers return codes

Signed-off-by: Karel Zak <kzak@redhat.com>

4 files changed, 191 insertions, 142 deletions

diff --git a/shlibs/mount/src/fs.c b/shlibs/mount/src/fs.c
index adf99ca4d..8c7baa266 100644
--- a/shlibs/mount/src/fs.c
+++ b/shlibs/mount/src/fs.c
@@ -189,24 +189,29 @@ const char *mnt_fs_get_source(mnt_fs *fs)
 }
 
 /* Used by parser mnt_file ONLY (@source has to be allocated) */
-int __mnt_fs_set_source(mnt_fs *fs, char *source)
+int __mnt_fs_set_source_ptr(mnt_fs *fs, char *source)
 {
+	char *t = NULL, *v = NULL;
+
 	assert(fs);
 
 	if (source && !strcmp(source, "none"))
 		source = NULL;
 
 	if (source && strchr(source, '=')) {
-		char *name, *val;
-
-		if (blkid_parse_tag_string(source, &name, &val) != 0)
+		if (blkid_parse_tag_string(source, &t, &v) != 0)
 			return -1;
-
-		fs->tagval = val;
-		fs->tagname = name;
 	}
 
+	if (fs->source != source)
+		free(fs->source);
+
+	free(fs->tagname);
+	free(fs->tagval);
+
 	fs->source = source;
+	fs->tagname = t;
+	fs->tagval = v;
 	return 0;
 }
 
@@ -222,6 +227,7 @@ int __mnt_fs_set_source(mnt_fs *fs, char *source)
 int mnt_fs_set_source(mnt_fs *fs, const char *source)
 {
 	char *p;
+	int rc;
 
 	if (!fs && !source)
 		return -EINVAL;
@@ -229,12 +235,10 @@ int mnt_fs_set_source(mnt_fs *fs, const char *source)
 	if (!p)
 		return -ENOMEM;
 
-	free(fs->tagval);
-	free(fs->tagname);
-	free(fs->source);
-	fs->tagval = fs->tagname = fs->source = NULL;
-
-	return __mnt_fs_set_source(fs, p);
+	rc = __mnt_fs_set_source_ptr(fs, p);
+	if (rc)
+		free(p);
+	return rc;
 }
 
 /**
@@ -332,13 +336,16 @@ const char *mnt_fs_get_fstype(mnt_fs *fs)
 }
 
 /* Used by mnt_file parser only */
-int __mnt_fs_set_fstype(mnt_fs *fs, char *fstype)
+int __mnt_fs_set_fstype_ptr(mnt_fs *fs, char *fstype)
 {
 	assert(fs);
 
 	if (!fstype)
 		return -EINVAL;
 
+	if (fstype != fs->fstype)
+		free(fs->fstype);
+
 	fs->fstype = fstype;
 	fs->flags &= ~MNT_FS_PSEUDO;
 	fs->flags &= ~MNT_FS_NET;
@@ -366,15 +373,17 @@ int __mnt_fs_set_fstype(mnt_fs *fs, char *fstype)
 int mnt_fs_set_fstype(mnt_fs *fs, const char *fstype)
 {
 	char *p;
+	int rc;
 
 	if (!fs || !fstype)
 		return -EINVAL;
 	p = strdup(fstype);
 	if (!p)
 		return -ENOMEM;
-	free(fs->fstype);
-
-	return __mnt_fs_set_fstype(fs, p);
+	rc =  __mnt_fs_set_fstype_ptr(fs, p);
+	if (rc)
+		free(p);
+	return rc;
 }
 
 /**
@@ -389,39 +398,51 @@ const char *mnt_fs_get_optstr(mnt_fs *fs)
 	return fs ? fs->optstr : NULL;
 }
 
-int __mnt_fs_set_optstr(mnt_fs *fs, const char *optstr, int split)
+int __mnt_fs_set_optstr_ptr(mnt_fs *fs, char *ptr, int split)
 {
-	char *p = NULL, *v = NULL, *f = NULL;
+	char *v = NULL, *f = NULL;
 
 	assert(fs);
 
 	if (!fs)
 		return -EINVAL;
-	if (optstr) {
+	if (ptr) {
 		int rc = 0;
 
 		if (split)
-			rc = mnt_split_optstr((char *) optstr, NULL, &v, &f, 0, 0);
+			rc = mnt_split_optstr((char *) ptr, NULL, &v, &f, 0, 0);
 		if (rc)
 			return rc;
-		p = strdup(optstr);
-		if (!p) {
-			free(v);
-			free(f);
-			return -ENOMEM;
-		}
 	}
 
-	free(fs->optstr);
+	if (ptr != fs->optstr)
+		free(fs->optstr);
+
 	free(fs->fs_optstr);
 	free(fs->vfs_optstr);
 
-	fs->optstr = p;
+	fs->optstr = ptr;
 	fs->fs_optstr = f;
 	fs->vfs_optstr = v;
 	return 0;
 }
 
+int __mnt_fs_set_optstr(mnt_fs *fs, const char *optstr, int split)
+{
+	char *p;
+	int rc;
+
+	assert(fs);
+
+	p = strdup(optstr);
+	if (!p)
+		return -ENOMEM;
+	rc = __mnt_fs_set_optstr_ptr(fs, p, split);
+	if (rc)
+		free(p);		/* error, deallocate */
+	return rc;
+}
+
 /**
  * mnt_fs_set_optstr:
  * @fs: fstab/mtab/mountinfo entry
diff --git a/shlibs/mount/src/mountP.h b/shlibs/mount/src/mountP.h
index be61885b8..9e8aa9dac 100644
--- a/shlibs/mount/src/mountP.h
+++ b/shlibs/mount/src/mountP.h
@@ -210,8 +210,9 @@ extern int mnt_optmap_require_value(const struct mnt_optmap *mapent);
 extern int mnt_optstr_remove_option_at(char **optstr, char *begin, char *end);
 
 /* fs.c */
-extern int __mnt_fs_set_source(mnt_fs *fs, char *source);
-extern int __mnt_fs_set_fstype(mnt_fs *fs, char *fstype);
+extern int __mnt_fs_set_source_ptr(mnt_fs *fs, char *source);
+extern int __mnt_fs_set_fstype_ptr(mnt_fs *fs, char *fstype);
+extern int __mnt_fs_set_optstr_ptr(mnt_fs *fs, char *optstr, int split);
 extern int __mnt_fs_set_optstr(mnt_fs *fs, const char *optstr, int split);
 
 #endif /* _LIBMOUNT_PRIVATE_H */
diff --git a/shlibs/mount/src/tab_parse.c b/shlibs/mount/src/tab_parse.c
index b9063c035..18f66505d 100644
--- a/shlibs/mount/src/tab_parse.c
+++ b/shlibs/mount/src/tab_parse.c
@@ -57,24 +57,24 @@ static size_t next_word_size(char *s, char **start, char **end)
 	return e - s;
 }
 
-static char *next_word(char **s)
+static int next_word(char **s, char **next)
 {
 	size_t sz;
-	char *res, *end;
+	char *end;
 
 	assert(s);
 
 	sz = next_word_size(*s, s, &end) + 1;
 	if (sz == 1)
-		return NULL;
+		return -EINVAL;
 
-	res = malloc(sz);
-	if (!res)
-		return NULL;
+	*next = malloc(sz);
+	if (!*next)
+		return -ENOMEM;
 
-	unmangle_to_buffer(*s, res, sz);
+	unmangle_to_buffer(*s, *next, sz);
 	*s = end + 1;
-	return res;
+	return 0;
 }
 
 static int next_word_skip(char **s)
@@ -115,47 +115,53 @@ static int next_number(char **s, int *num)
  */
 static int mnt_tab_parse_file_line(mnt_fs *fs, char *s)
 {
-	int rc = 0;
-	char *p = NULL;
-
-	/* SOURCE */
-	rc =__mnt_fs_set_source(fs, next_word(&s));
-	if (rc)
-		goto err;
+	int rc = 0, col;
 
-	/* TARGET */
-	fs->target = next_word(&s);
-	if (!fs->target)
-		goto err;
+	fs->passno = fs->freq = 0;	/* default */
 
-	/* TYPE */
-	rc = __mnt_fs_set_fstype(fs, next_word(&s));
-	if (rc)
-		goto err;
+	for (col = 0; rc == 0 && col < 4; col++) {
+		char *p = NULL;
 
-	/* OPTS */
-	p = next_word(&s);
-	if (!p || mnt_fs_set_optstr(fs, p))
-		goto err;
+		rc = next_word(&s, &p);
+		if (rc)
+			goto done;
 
-	/* default */
-	fs->passno = fs->freq = 0;
+		switch(col) {
+		/* SOURCE */
+		case 0:
+			rc =__mnt_fs_set_source_ptr(fs, p);
+			break;
+		/* TARGET */
+		case 1:
+			fs->target = p;
+			break;
+		/* TYPE */
+		case 2:
+			rc = __mnt_fs_set_fstype_ptr(fs, p);
+			break;
+		/* OPTS */
+		case 3:
+			rc = __mnt_fs_set_optstr_ptr(fs, p, TRUE);
+			break;
+		}
+		if (rc) {
+			free(p);
+			goto done;
+		}
+	}
 
 	/* FREQ (optional) */
-	if (next_number(&s, &fs->freq) != 0) {
-		if (*s)
-			goto err;;
-
-	/* PASSNO (optional) */
-	} else if (next_number(&s, &fs->passno) != 0 && *s)
-		goto err;
-
-	return 0;
-err:
-	free(p);
-	if (rc)
-		return rc;
-	return errno == ENOMEM ? -ENOMEM : -EINVAL;
+	rc = next_number(&s, &fs->freq);
+	if (rc && !*s)
+		rc = 0;		/* no error, end of line, freq is optional */
+
+	else {
+		rc = next_number(&s, &fs->passno);
+		if (rc && !*s)
+			rc = 0;
+	}
+done:
+	return rc;
 }
 
 /*
@@ -163,76 +169,86 @@ err:
  */
 static int mnt_parse_mountinfo_line(mnt_fs *fs, char *s)
 {
+	int rc = 0, col;
 	unsigned int maj, min;
-	int rc = 0;
+	char *p = NULL;
 
 	/* ID */
-	if (next_number(&s, &fs->id) != 0)
-		goto err;
+	rc = next_number(&s, &fs->id);
+	if (rc)
+		goto done;
 
 	/* PARENT */
-	if (next_number(&s, &fs->parent) != 0)
-		goto err;
+	rc = next_number(&s, &fs->parent);
+	if (rc)
+		goto done;
 
 	/* <maj>:<min> */
 	s = skip_spaces(s);
 	if (!*s || sscanf(s, "%u:%u", &maj, &min) != 2)
-		goto err;
-
-	fs->devno = makedev(maj, min);
-	next_word_skip(&s);
+		rc = -EINVAL;
+	else {
+		fs->devno = makedev(maj, min);
+		next_word_skip(&s);
+	}
 
-	/* MOUNTROOT */
-	fs->root = next_word(&s);
-	if (!fs->root)
-		goto err;
+	for (col = 3; rc == 0 && col < 9; col++) {
+		rc = next_word(&s, &p);
+		if (rc)
+			break;
 
-	/* TARGET (mountpoit) */
-	fs->target = next_word(&s);
-	if (!fs->target)
-		goto err;
+		switch(col) {
+		/* MOUNTROOT */
+		case 3:
+			fs->root = p;
+			break;
 
-	/* OPTIONS (fs-independent) */
-	fs->vfs_optstr = next_word(&s);
-	if (!fs->vfs_optstr)
-		goto err;
+		/* TARGET (mountpoit) */
+		case 4:
+			fs->target = p;
+			break;
 
-	/* optional fields (ignore) */
-	do {
-		s = skip_spaces(s);
-		if (s && *s == '-' &&
-		    (*(s + 1) == ' ' || *(s + 1) == '\t')) {
-			s++;
+		/* OPTIONS (fs-independent) */
+		case 5:
+			fs->vfs_optstr = p;
+
+			/* ignore optional fields behind options */
+			do {
+				s = skip_spaces(s);
+				if (s && *s == '-' &&
+				    (*(s + 1) == ' ' || *(s + 1) == '\t')) {
+					s++;
+					break;
+				}
+				next_word_skip(&s);
+			} while (s);
 			break;
-		}
-		if (s && next_word_skip(&s) != 0)
-			goto err;
-	} while (s);
 
-	/* FSTYPE */
-	rc =__mnt_fs_set_fstype(fs, next_word(&s));
-	if (rc)
-		goto err;
+		/* FSTYPE */
+		case 6:
+			rc =__mnt_fs_set_fstype_ptr(fs, p);
+			break;
 
-	/* SOURCE or "none" */
-	rc = __mnt_fs_set_source(fs, next_word(&s));
-	if (rc)
-		goto err;
+		/* SOURCE or "none" */
+		case 7:
+			rc = __mnt_fs_set_source_ptr(fs, p);
+			break;
 
-	/* OPTIONS (fs-dependent) */
-	fs->fs_optstr = next_word(&s);
-	if (!fs->fs_optstr)
-		goto err;
+		/* OPTIONS (fs-dependent) */
+		case 8:
+			fs->fs_optstr = p;
 
-	if (!strcmp(fs->fs_optstr, "none")) {
-		free(fs->fs_optstr);
-		fs->fs_optstr = NULL;
+			if (!strcmp(fs->fs_optstr, "none")) {
+				free(fs->fs_optstr);
+				fs->fs_optstr = NULL;
+			}
+			break;
+		}
 	}
-	return 0;
-err:
 	if (rc)
-		return rc;
-	return errno == ENOMEM ? -ENOMEM : -EINVAL;
+		free(p);
+done:
+	return rc;
 }
 
 /*
diff --git a/shlibs/mount/src/tab_update.c b/shlibs/mount/src/tab_update.c
index 1a56c44c2..3cf676211 100644
--- a/shlibs/mount/src/tab_update.c
+++ b/shlibs/mount/src/tab_update.c
@@ -533,18 +533,19 @@ err:
  * This function has to be always called before mount(2). The mnt_update_file()
  * should not be called if mnt_prepare_update() returns non-zero value.
  *
- * Returns: 0 on success, 1 if update is unnecessary, -1 in case of error
+ * Returns: 0 on success, 1 if update is unnecessary, negative in case of error
  */
 int mnt_prepare_update(mnt_update *upd)
 {
 	char *u = NULL;
 	const char *o = NULL;
+	int rc = 0;
 
 	assert(upd);
 	assert(upd->fs);
 
 	if (!upd || !upd->fs)
-		return -1;
+		return -EINVAL;
 
 	DBG(UPDATE, mnt_debug_h(upd,
 		"prepare update (target %s, source %s, optstr %s)",
@@ -555,13 +556,17 @@ int mnt_prepare_update(mnt_update *upd)
 	if (!upd->filename) {
 		const char *p = mnt_get_writable_mtab_path();
 		if (!p) {
-			if (errno)
-			       goto err;	/* EACCES? */
+			if (errno) {
+				rc = -errno;
+				goto err;	/* EACCES? */
+			}
 			goto nothing;		/* no mtab */
 		}
 		upd->filename = strdup(p);
-		if (!upd->filename)
+		if (!upd->filename) {
+			rc = -ENOMEM;
 			goto err;
+		}
 	}
 	if (!upd->format) {
 		if (endswith(upd->filename, "mountinfo"))
@@ -597,39 +602,45 @@ int mnt_prepare_update(mnt_update *upd)
 	 *  - remove all non-userspace mount options
 	 */
 	if (upd->mountflags & MS_REMOUNT) {
-		if (mnt_split_optstr(o, &u, NULL, NULL, MNT_NOMTAB, 0))
+		rc = mnt_split_optstr(o, &u, NULL, NULL, MNT_NOMTAB, 0);
+		if (rc)
 			goto err;
-		if (__mnt_fs_set_optstr(upd->fs, u, FALSE))
+		rc = __mnt_fs_set_optstr_ptr(upd->fs, u, FALSE);
+		if (rc)
 			goto err;
-
+		u = NULL;
 	} else {
 		if (!o)
 			goto nothing;	/* no options */
-		if (mnt_split_optstr(o, &u, NULL, NULL, MNT_NOMTAB, 0))
+		rc = mnt_split_optstr(o, &u, NULL, NULL, MNT_NOMTAB, 0);
+		if (rc)
 			goto err;
 		if (!u)
 			goto nothing;	/* no userpsace options */
-		if (set_fs_root(upd, upd->fs))
+		rc = set_fs_root(upd, upd->fs);
+		if (rc)
 			goto err;
-		__mnt_fs_set_optstr(upd->fs, u, FALSE);
+		rc = __mnt_fs_set_optstr_ptr(upd->fs, u, FALSE);
+		if (rc)
+			goto err;
+		u = NULL;
 	}
 
 	if (!upd->nolock && !upd->lc) {
 		upd->lc = mnt_new_lock(upd->filename, 0);
-		if (!upd->lc)
+		if (!upd->lc) {
+			rc = -ENOMEM;
 			goto err;
+		}
 	}
 
 	DBG(UPDATE, mnt_debug_h(upd, "prepare update: success"));
-	free(u);
 	return 0;
 err:
-	DBG(UPDATE, mnt_debug_h(upd, "prepare update: failed"));
 	free(u);
-	return -1;
+	DBG(UPDATE, mnt_debug_h(upd, "prepare update: failed"));
+	return rc;
 nothing:
-	DBG(UPDATE, mnt_debug_h(upd, "prepare update: unnecessary"));
-	free(u);
 	return 1;
 }
 
@@ -759,7 +770,7 @@ static int modify_options(mnt_update *upd)
 		mnt_tab_remove_fs(tb, fs);
 		rem_fs = fs;
 	} else
-		__mnt_fs_set_optstr(fs, mnt_fs_get_optstr(upd->fs), FALSE);
+		rc = __mnt_fs_set_optstr(fs, mnt_fs_get_optstr(upd->fs), FALSE);
 
 	if (!update_file(upd->filename, upd->format, tb))
 		rc = 0;