[pacman-dev] [PATCH] fixed erroneous memory access to newurl in alpm_db_remove_server
Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com> --- lib/libalpm/db.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index bf9c3f0..26b6250 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -198,16 +198,20 @@ int SYMEXPORT alpm_db_remove_server(alpm_db_t *db, const char *url) if(!newurl) { return -1; } + db->servers = alpm_list_remove_str(db->servers, newurl, &vdata); - free(newurl); + + int ret = 1; + if(vdata) { _alpm_log(db->handle, ALPM_LOG_DEBUG, "removed server URL from database '%s': %s\n", db->treename, newurl); free(vdata); - return 0; + ret = 0; } - return 1; + free(newurl); + return ret; } /** Get the name of a package database. */ -- Barbu Paul - Gheorghe Common sense is not so common - Voltaire Visit My GitHub profile to see my open-source projects - https://github.com/paullik
On 31/07/12 05:57, Barbu Paul - Gheorghe wrote:
Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com> --- lib/libalpm/db.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index bf9c3f0..26b6250 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -198,16 +198,20 @@ int SYMEXPORT alpm_db_remove_server(alpm_db_t *db, const char *url) if(!newurl) { return -1; } + db->servers = alpm_list_remove_str(db->servers, newurl, &vdata); - free(newurl); + + int ret = 1;
Our current coding standard is to declare all variables at the start of the block.
if(vdata) { _alpm_log(db->handle, ALPM_LOG_DEBUG, "removed server URL from database '%s': %s\n", db->treename, newurl); free(vdata);
Instead of creating a new variable, why not just free(newurl) here and keep the return?
- return 0; + ret = 0; }
- return 1; + free(newurl); + return ret; }
/** Get the name of a package database. */
Our current coding standard is to declare all variables at the start of the block.
if(vdata) { _alpm_log(db->handle, ALPM_LOG_DEBUG, "removed server URL from database '%s': %s\n", db->treename, newurl); free(vdata);
Instead of creating a new variable, why not just free(newurl) here and keep the return?
The reason behind this is that I didn't want to have two calls to free(newurl) for each execution branch (the same applies for the return statement). Should I modify it with two calls or should I leave it this way and just fix the coding standard violation that you mentioned above? Have a nice day! -- Barbu Paul - Gheorghe Common sense is not so common - Voltaire Visit My GitHub profile to see my open-source projects - https://github.com/paullik
On 07/31/2012 09:01 AM, Barbu Paul - Gheorghe wrote:
Instead of creating a new variable, why not just free(newurl) here and keep the return?
The reason behind this is that I didn't want to have two calls to free(newurl) for each execution branch (the same applies for the return statement).
Should I modify it with two calls or should I leave it this way and just fix the coding standard violation that you mentioned above?
Have a nice day!
What should I do? Did anyone looked into my argument yet? -- Barbu Paul - Gheorghe Common sense is not so common - Voltaire Visit My GitHub profile to see my open-source projects - https://github.com/paullik
On 08/08/12 18:00, Barbu Paul - Gheorghe wrote:
On 07/31/2012 09:01 AM, Barbu Paul - Gheorghe wrote:
Instead of creating a new variable, why not just free(newurl) here and keep the return?
The reason behind this is that I didn't want to have two calls to free(newurl) for each execution branch (the same applies for the return statement).
Should I modify it with two calls or should I leave it this way and just fix the coding standard violation that you mentioned above?
Have a nice day!
What should I do? Did anyone looked into my argument yet?
@Dan: what is your preference here? Allan
participants (2)
-
Allan McRae
-
Barbu Paul - Gheorghe