config: Refactor configuration file parsing

Submitted by Radostin Stoyanov on Jan. 25, 2019, 9:27 p.m.

Details

Message ID 20190125212727.566-1-rstoyanov1@gmail.com
State Accepted
Series "config: Refactor configuration file parsing"
Headers show

Commit Message

Radostin Stoyanov Jan. 25, 2019, 9:27 p.m.
Split the function parse_config() into smaller parts by introducing
the parse_statement(), and add more descriptive comments to improve
readability. In addition, make sure that the last element of the array
of strings 'configuration' is initialised to NULL, which is necessary
to properly count the number of elements after parsing.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/config.c | 164 +++++++++++++++++++++++++++-----------------------
 1 file changed, 89 insertions(+), 75 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/config.c b/criu/config.c
index b6ecbfb64..1a41b4533 100644
--- a/criu/config.c
+++ b/criu/config.c
@@ -40,20 +40,91 @@  static int count_elements(char **to_count)
 	return count;
 }
 
+/* Parse one statement in configuration file */
+int parse_statement(int i, char *line, char **configuration)
+{
+	int offset = 0, len = 0;
+	bool was_newline = true;
+	char *tmp_string, *quoted, *quotedptr;
+
+	while (1) {
+		/* Ignore white-space */
+		while ((isspace(*(line + offset)) && (*(line + offset) != '\n'))) offset++;
+
+		/* Read a single word. A word is everything
+		 * that doesn't contain white-space characters. */
+		if (sscanf(line + offset, "%m[^ \t\n]s", &configuration[i]) != 1) {
+			configuration[i] = NULL;
+			break;
+		}
+
+		/* Ignore comments - everything between '#' and '\n' */
+		if (configuration[i][0] == '#') {
+			configuration[i] = NULL;
+			break;
+		}
+
+		if ((configuration[i][0] == '\"') && (strchr(line + offset + 1, '"'))) {
+			/* Handle empty strings which strtok ignores */
+			if (!strcmp(configuration[i], "\"\"")) {
+				configuration[i] = "";
+				offset += strlen("\"\"");
+			} else if ((configuration[i] = strtok_r(line + offset, "\"", &quotedptr))) {
+				/* Handle escaping of quotes in quoted string */
+				while (configuration[i][strlen(configuration[i]) - 1] == '\\') {
+					offset++;
+					len = strlen(configuration[i]);
+					configuration[i][len - 1] = '"';
+					if (*quotedptr == '"') {
+						quotedptr++;
+						break;
+					}
+					quoted = strtok_r(NULL, "\"", &quotedptr);
+					tmp_string = xmalloc(len + strlen(quoted) + 1);
+					if (tmp_string == NULL)
+						return -1;
+
+					memmove(tmp_string, configuration[i], len);
+					memmove(tmp_string + len, quoted, strlen(quoted) + 1);
+					configuration[i] = tmp_string;
+				}
+				offset += 2;
+			}
+		}
+
+		offset += strlen(configuration[i]);
+
+		if (was_newline) {
+			was_newline = false;
+			len = strlen(configuration[i]);
+			tmp_string = xrealloc(configuration[i], len + strlen("--") + 1);
+			if (tmp_string == NULL)
+				return -1;
+
+			memmove(tmp_string + strlen("--"), tmp_string, len + 1);
+			memmove(tmp_string, "--", strlen("--"));
+			configuration[i] = tmp_string;
+		}
+		i++;
+	}
+
+	return i;
+}
+
+/* Parse a configuration file */
 static char ** parse_config(char *filepath)
 {
 #define DEFAULT_CONFIG_SIZE	10
 	FILE* configfile = fopen(filepath, "r");
 	int config_size = DEFAULT_CONFIG_SIZE;
-	int i = 1, len = 0, offset;
-	size_t limit = 0;
-	bool was_newline;
-	char *tmp_string, *line = NULL, *quoted, *quotedptr;
-	char **configuration, **tmp_conf;
+	int i = 1;
+	size_t line_size = 0;
+	char *line = NULL;
+	char **configuration;
 
-	if (!configfile) {
+	if (!configfile)
 		return NULL;
-	}
+
 	configuration = xmalloc(config_size * sizeof(char *));
 	if (configuration == NULL) {
 		fclose(configfile);
@@ -64,85 +135,28 @@  static char ** parse_config(char *filepath)
 	 */
 	configuration[0] = "criu";
 
-	while ((len = getline(&line, &limit, configfile)) != -1) {
-		offset = 0;
-		was_newline = true;
+	while (getline(&line, &line_size, configfile) != -1) {
+		/* Extend configuration buffer if necessary */
 		if (i >= config_size - 1) {
 			config_size *= 2;
-			tmp_conf = xrealloc(configuration, config_size * sizeof(char *));
-			if (tmp_conf == NULL) {
+			configuration = xrealloc(configuration, config_size * sizeof(char *));
+			if (configuration == NULL) {
 				fclose(configfile);
 				exit(1);
 			}
-			configuration = tmp_conf;
 		}
-		while (1) {
-			while ((isspace(*(line + offset)) && (*(line + offset) != '\n'))) offset++;
-
-			if (sscanf(line + offset, "%m[^ \t\n]s", &configuration[i]) != 1) {
-				configuration[i] = NULL;
-				break;
-			}
 
-			if (configuration[i][0] == '#') {
-				if (sscanf(line, "%*[^\n]") != 0) {
-					pr_err("Error while reading configuration file %s\n", filepath);
-					fclose(configfile);
-					exit(1);
-				}
-				configuration[i] = NULL;
-				break;
-			}
-			if ((configuration[i][0] == '\"') && (strchr(line + offset + 1, '"'))) {
-				/*
-				 * Handle empty strings which strtok ignores
-				 */
-				if (!strcmp(configuration[i], "\"\"")) {
-					configuration[i] = "";
-					offset += strlen("\"\"");
-				} else if ((configuration[i] = strtok_r(line + offset, "\"", &quotedptr))) {
-					/*
-					 * Handle escaping of quotes in quoted string
-					 */
-					while (configuration[i][strlen(configuration[i]) - 1] == '\\') {
-						offset++;
-						len = strlen(configuration[i]);
-						configuration[i][len - 1] = '"';
-						if (*quotedptr == '"') {
-							quotedptr++;
-							break;
-						}
-						quoted = strtok_r(NULL, "\"", &quotedptr);
-						tmp_string = xmalloc(len + strlen(quoted) + 1);
-						if (tmp_string == NULL) {
-							fclose(configfile);
-							exit(1);
-						}
-						memmove(tmp_string, configuration[i], len);
-						memmove(tmp_string + len, quoted, strlen(quoted) + 1);
-						configuration[i] = tmp_string;
-					}
-					offset += 2;
-				}
-			}
-			offset += strlen(configuration[i]);
-			if (was_newline) {
-				was_newline = false;
-				len = strlen(configuration[i]);
-				tmp_string = xrealloc(configuration[i], len + strlen("--") + 1);
-				if (tmp_string == NULL) {
-					fclose(configfile);
-					exit(1);
-				}
-				memmove(tmp_string + strlen("--"), tmp_string, len + 1);
-				memmove(tmp_string, "--", strlen("--"));
-				configuration[i] = tmp_string;
-			}
-			i++;
+		i = parse_statement(i, line, configuration);
+		if (i < 0) {
+			fclose(configfile);
+			exit(1);
 		}
+
 		free(line);
 		line = NULL;
 	}
+	/* Initialize the last element */
+	configuration[i] = NULL;
 
 	free(line);
 	fclose(configfile);

Comments

Andrei Vagin Feb. 1, 2019, 5:51 p.m.
Accepted, thanks!

On Fri, Jan 25, 2019 at 09:27:27PM +0000, Radostin Stoyanov wrote:
> Split the function parse_config() into smaller parts by introducing
> the parse_statement(), and add more descriptive comments to improve
> readability. In addition, make sure that the last element of the array
> of strings 'configuration' is initialised to NULL, which is necessary
> to properly count the number of elements after parsing.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/config.c | 164 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 89 insertions(+), 75 deletions(-)
> 
> diff --git a/criu/config.c b/criu/config.c
> index b6ecbfb64..1a41b4533 100644
> --- a/criu/config.c
> +++ b/criu/config.c
> @@ -40,20 +40,91 @@ static int count_elements(char **to_count)
>  	return count;
>  }
>  
> +/* Parse one statement in configuration file */
> +int parse_statement(int i, char *line, char **configuration)
> +{
> +	int offset = 0, len = 0;
> +	bool was_newline = true;
> +	char *tmp_string, *quoted, *quotedptr;
> +
> +	while (1) {
> +		/* Ignore white-space */
> +		while ((isspace(*(line + offset)) && (*(line + offset) != '\n'))) offset++;
> +
> +		/* Read a single word. A word is everything
> +		 * that doesn't contain white-space characters. */
> +		if (sscanf(line + offset, "%m[^ \t\n]s", &configuration[i]) != 1) {
> +			configuration[i] = NULL;
> +			break;
> +		}
> +
> +		/* Ignore comments - everything between '#' and '\n' */
> +		if (configuration[i][0] == '#') {
> +			configuration[i] = NULL;
> +			break;
> +		}
> +
> +		if ((configuration[i][0] == '\"') && (strchr(line + offset + 1, '"'))) {
> +			/* Handle empty strings which strtok ignores */
> +			if (!strcmp(configuration[i], "\"\"")) {
> +				configuration[i] = "";
> +				offset += strlen("\"\"");
> +			} else if ((configuration[i] = strtok_r(line + offset, "\"", &quotedptr))) {
> +				/* Handle escaping of quotes in quoted string */
> +				while (configuration[i][strlen(configuration[i]) - 1] == '\\') {
> +					offset++;
> +					len = strlen(configuration[i]);
> +					configuration[i][len - 1] = '"';
> +					if (*quotedptr == '"') {
> +						quotedptr++;
> +						break;
> +					}
> +					quoted = strtok_r(NULL, "\"", &quotedptr);
> +					tmp_string = xmalloc(len + strlen(quoted) + 1);
> +					if (tmp_string == NULL)
> +						return -1;
> +
> +					memmove(tmp_string, configuration[i], len);
> +					memmove(tmp_string + len, quoted, strlen(quoted) + 1);
> +					configuration[i] = tmp_string;
> +				}
> +				offset += 2;
> +			}
> +		}
> +
> +		offset += strlen(configuration[i]);
> +
> +		if (was_newline) {
> +			was_newline = false;
> +			len = strlen(configuration[i]);
> +			tmp_string = xrealloc(configuration[i], len + strlen("--") + 1);
> +			if (tmp_string == NULL)
> +				return -1;
> +
> +			memmove(tmp_string + strlen("--"), tmp_string, len + 1);
> +			memmove(tmp_string, "--", strlen("--"));
> +			configuration[i] = tmp_string;
> +		}
> +		i++;
> +	}
> +
> +	return i;
> +}
> +
> +/* Parse a configuration file */
>  static char ** parse_config(char *filepath)
>  {
>  #define DEFAULT_CONFIG_SIZE	10
>  	FILE* configfile = fopen(filepath, "r");
>  	int config_size = DEFAULT_CONFIG_SIZE;
> -	int i = 1, len = 0, offset;
> -	size_t limit = 0;
> -	bool was_newline;
> -	char *tmp_string, *line = NULL, *quoted, *quotedptr;
> -	char **configuration, **tmp_conf;
> +	int i = 1;
> +	size_t line_size = 0;
> +	char *line = NULL;
> +	char **configuration;
>  
> -	if (!configfile) {
> +	if (!configfile)
>  		return NULL;
> -	}
> +
>  	configuration = xmalloc(config_size * sizeof(char *));
>  	if (configuration == NULL) {
>  		fclose(configfile);
> @@ -64,85 +135,28 @@ static char ** parse_config(char *filepath)
>  	 */
>  	configuration[0] = "criu";
>  
> -	while ((len = getline(&line, &limit, configfile)) != -1) {
> -		offset = 0;
> -		was_newline = true;
> +	while (getline(&line, &line_size, configfile) != -1) {
> +		/* Extend configuration buffer if necessary */
>  		if (i >= config_size - 1) {
>  			config_size *= 2;
> -			tmp_conf = xrealloc(configuration, config_size * sizeof(char *));
> -			if (tmp_conf == NULL) {
> +			configuration = xrealloc(configuration, config_size * sizeof(char *));
> +			if (configuration == NULL) {
>  				fclose(configfile);
>  				exit(1);
>  			}
> -			configuration = tmp_conf;
>  		}
> -		while (1) {
> -			while ((isspace(*(line + offset)) && (*(line + offset) != '\n'))) offset++;
> -
> -			if (sscanf(line + offset, "%m[^ \t\n]s", &configuration[i]) != 1) {
> -				configuration[i] = NULL;
> -				break;
> -			}
>  
> -			if (configuration[i][0] == '#') {
> -				if (sscanf(line, "%*[^\n]") != 0) {
> -					pr_err("Error while reading configuration file %s\n", filepath);
> -					fclose(configfile);
> -					exit(1);
> -				}
> -				configuration[i] = NULL;
> -				break;
> -			}
> -			if ((configuration[i][0] == '\"') && (strchr(line + offset + 1, '"'))) {
> -				/*
> -				 * Handle empty strings which strtok ignores
> -				 */
> -				if (!strcmp(configuration[i], "\"\"")) {
> -					configuration[i] = "";
> -					offset += strlen("\"\"");
> -				} else if ((configuration[i] = strtok_r(line + offset, "\"", &quotedptr))) {
> -					/*
> -					 * Handle escaping of quotes in quoted string
> -					 */
> -					while (configuration[i][strlen(configuration[i]) - 1] == '\\') {
> -						offset++;
> -						len = strlen(configuration[i]);
> -						configuration[i][len - 1] = '"';
> -						if (*quotedptr == '"') {
> -							quotedptr++;
> -							break;
> -						}
> -						quoted = strtok_r(NULL, "\"", &quotedptr);
> -						tmp_string = xmalloc(len + strlen(quoted) + 1);
> -						if (tmp_string == NULL) {
> -							fclose(configfile);
> -							exit(1);
> -						}
> -						memmove(tmp_string, configuration[i], len);
> -						memmove(tmp_string + len, quoted, strlen(quoted) + 1);
> -						configuration[i] = tmp_string;
> -					}
> -					offset += 2;
> -				}
> -			}
> -			offset += strlen(configuration[i]);
> -			if (was_newline) {
> -				was_newline = false;
> -				len = strlen(configuration[i]);
> -				tmp_string = xrealloc(configuration[i], len + strlen("--") + 1);
> -				if (tmp_string == NULL) {
> -					fclose(configfile);
> -					exit(1);
> -				}
> -				memmove(tmp_string + strlen("--"), tmp_string, len + 1);
> -				memmove(tmp_string, "--", strlen("--"));
> -				configuration[i] = tmp_string;
> -			}
> -			i++;
> +		i = parse_statement(i, line, configuration);
> +		if (i < 0) {
> +			fclose(configfile);
> +			exit(1);
>  		}
> +
>  		free(line);
>  		line = NULL;
>  	}
> +	/* Initialize the last element */
> +	configuration[i] = NULL;
>  
>  	free(line);
>  	fclose(configfile);
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu