[pacman-dev] [PATCH] Give an error message on alpm_db_register_sync() error
From 8e11e0c7198f7c7bdeb6ca76d647d4831593b965 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Mon, 17 Nov 2008 17:02:43 +0100 Subject: [PATCH] Give an error message on alpm_db_register_sync() error
This patch slightly modifies pacman.c/_parseconfig():
See FS#12148. Now pacman prints the following error message in that case: "error: could not register 'unstable' database (could not open database)"
I also added an error message for alpm_db_setserver() error.
I changed the "return(1);" scheme to "ret = 1; goto clean_up;" to make sure that we free allocated memory and close open files.
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- src/pacman/pacman.c | 46 ++++++++++++++++++++++++++++++++++------------ 1 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 3813a16..6d588f8 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -583,6 +583,7 @@ static int _parseconfig(const char *file, const char *givensection, int linenum = 0; char *ptr, *section = NULL; pmdb_t *db = NULL; + int ret = 0;
pm_printf(PM_LOG_DEBUG, "config: attempting to read file %s\n", file); fp = fopen(file, "r"); @@ -625,7 +626,8 @@ static int _parseconfig(const char *file, const char *givensection, if(!strlen(section)) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: bad section name.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up; } /* if we are not looking at the options section, register a db and also * ensure we have set all of our library paths as the library is too stupid @@ -633,6 +635,12 @@ static int _parseconfig(const char *file, const char *givensection, if(strcmp(section, "options") != 0) { setlibpaths(); db = alpm_db_register_sync(section); + if(db == NULL) { + pm_printf(PM_LOG_ERROR, _("could not register '%s' database (%s)\n"), + section, alpm_strerrorlast()); + ret = 1; + goto clean_up; + } } } else { /* directive */ @@ -647,13 +655,15 @@ static int _parseconfig(const char *file, const char *givensection, if(key == NULL) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: syntax error in config file- missing key.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up; } /* For each directive, compare to the camelcase string. */ if(section == NULL) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: All directives must belong to a section.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up; } if(ptr == NULL && strcmp(section, "options") == 0) { /* directives without settings, all in [options] */ @@ -678,7 +688,8 @@ static int _parseconfig(const char *file, const char *givensection, } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } else { /* directives with settings */ @@ -709,7 +720,8 @@ static int _parseconfig(const char *file, const char *givensection, if(alpm_option_add_cachedir(ptr) != 0) { pm_printf(PM_LOG_ERROR, _("problem adding cachedir '%s' (%s)\n"), ptr, alpm_strerrorlast()); - return(1); + ret = 1; + goto clean_up; } pm_printf(PM_LOG_DEBUG, "config: cachedir: %s\n", ptr); } else if(strcmp(key, "RootDir") == 0) { @@ -733,13 +745,15 @@ static int _parseconfig(const char *file, const char *givensection, config->cleanmethod = PM_CLEAN_KEEPCUR; } else { pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); - return(1); + ret = 1; + goto clean_up; } pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } else if(strcmp(key, "Server") == 0) { /* let's attempt a replacement for the current repo */ @@ -747,27 +761,35 @@ static int _parseconfig(const char *file, const char *givensection,
if(alpm_db_setserver(db, server) != 0) { /* pm_errno is set by alpm_db_setserver */ - return(1); + pm_printf(PM_LOG_ERROR, _("could not add server URL to database '%s': %s (%s)\n"), + alpm_db_get_name(db), server, alpm_strerrorlast()); + free(server); + ret = 1; + goto clean_up; }
free(server); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } } } - fclose(fp); + +clean_up: + if(fp) { + fclose(fp); + } if(section){ free(section); } - /* call setlibpaths here to ensure we have called it at least once */ setlibpaths(); pm_printf(PM_LOG_DEBUG, "config: finished parsing %s\n", file); - return(0); + return(ret); }
/** Parse a configuration file. -- 1.6.0.3
Omg, you can find my patch here: http://repo.or.cz/w/pacman-ng.git Bye
On Mon, Nov 17, 2008 at 5:33 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
From 8e11e0c7198f7c7bdeb6ca76d647d4831593b965 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Mon, 17 Nov 2008 17:02:43 +0100 Subject: [PATCH] Give an error message on alpm_db_register_sync() error
This patch slightly modifies pacman.c/_parseconfig():
See FS#12148. Now pacman prints the following error message in that case: "error: could not register 'unstable' database (could not open database)"
I also added an error message for alpm_db_setserver() error.
I changed the "return(1);" scheme to "ret = 1; goto clean_up;" to make sure that we free allocated memory and close open files.
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- src/pacman/pacman.c | 46 ++++++++++++++++++++++++++++++++++------------ 1 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 3813a16..6d588f8 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -583,6 +583,7 @@ static int _parseconfig(const char *file, const char *givensection, int linenum = 0; char *ptr, *section = NULL; pmdb_t *db = NULL; + int ret = 0;
pm_printf(PM_LOG_DEBUG, "config: attempting to read file %s\n", file); fp = fopen(file, "r"); @@ -625,7 +626,8 @@ static int _parseconfig(const char *file, const char *givensection, if(!strlen(section)) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: bad section name.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up;
Can we use "cleanup" as the label instead? It is this way every other place in the code, consistency would be a good thing.
} /* if we are not looking at the options section, register a db and also * ensure we have set all of our library paths as the library is too stupid @@ -633,6 +635,12 @@ static int _parseconfig(const char *file, const char *givensection, if(strcmp(section, "options") != 0) { setlibpaths(); db = alpm_db_register_sync(section); + if(db == NULL) { + pm_printf(PM_LOG_ERROR, _("could not register '%s' database (%s)\n"), + section, alpm_strerrorlast());
Obviously an error is better than no error at all, but does this have meaning to end users at all? I don't know what else to put however, so for now it is fine.
+ ret = 1; + goto clean_up; + } } } else { /* directive */ @@ -647,13 +655,15 @@ static int _parseconfig(const char *file, const char *givensection, if(key == NULL) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: syntax error in config file- missing key.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up; } /* For each directive, compare to the camelcase string. */ if(section == NULL) { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: All directives must belong to a section.\n"), file, linenum); - return(1); + ret = 1; + goto clean_up; } if(ptr == NULL && strcmp(section, "options") == 0) { /* directives without settings, all in [options] */ @@ -678,7 +688,8 @@ static int _parseconfig(const char *file, const char *givensection, } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } else { /* directives with settings */ @@ -709,7 +720,8 @@ static int _parseconfig(const char *file, const char *givensection, if(alpm_option_add_cachedir(ptr) != 0) { pm_printf(PM_LOG_ERROR, _("problem adding cachedir '%s' (%s)\n"), ptr, alpm_strerrorlast()); - return(1); + ret = 1; + goto clean_up; } pm_printf(PM_LOG_DEBUG, "config: cachedir: %s\n", ptr); } else if(strcmp(key, "RootDir") == 0) { @@ -733,13 +745,15 @@ static int _parseconfig(const char *file, const char *givensection, config->cleanmethod = PM_CLEAN_KEEPCUR; } else { pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); - return(1); + ret = 1; + goto clean_up; } pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } else if(strcmp(key, "Server") == 0) { /* let's attempt a replacement for the current repo */ @@ -747,27 +761,35 @@ static int _parseconfig(const char *file, const char *givensection,
if(alpm_db_setserver(db, server) != 0) { /* pm_errno is set by alpm_db_setserver */ - return(1); + pm_printf(PM_LOG_ERROR, _("could not add server URL to database '%s': %s (%s)\n"), + alpm_db_get_name(db), server, alpm_strerrorlast()); + free(server); + ret = 1; + goto clean_up; We didn't do so great with memory leaks before. :)
}
free(server); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); - return(1); + ret = 1; + goto clean_up; } } } } - fclose(fp); + +clean_up: + if(fp) { + fclose(fp); + } if(section){ free(section); } - /* call setlibpaths here to ensure we have called it at least once */ setlibpaths(); pm_printf(PM_LOG_DEBUG, "config: finished parsing %s\n", file); - return(0); + return(ret); }
/** Parse a configuration file. --
Looks good outside of my main "cleanup" naming comment. I'll pull this into maint once that gets addressed, thanks for the quick fix to the missing message and Flyspray bug. -Dan
Looks good outside of my main "cleanup" naming comment. I'll pull this into maint once that gets addressed, thanks for the quick fix to the missing message and Flyspray bug.
OK. Fixed in my repo. (In fact I used clean_up label because we already have a cleanup function in pacman.c.) Bye
participants (2)
-
Dan McGee
-
Nagy Gabor