On Mon, Aug 08, 2011 at 12:29:23PM -0500, Dan McGee wrote:
> We did a good job checking this in add.c, but not necessarily anywhere
> else. Fix this up by adding checks into dload.c, remove.c, and conf.c in
> the frontend. Also add loggers where appropriate and make the message
> syntax more consistent.
>
> Signed-off-by: Dan McGee <[email protected]>
> ---
>
> Dave- any memories/reasoning as to why you didn't port this bit over when you
> added the curl backend stuff? (commit 96e458b705eda4ddff)
Laziness? Bad form? I wouldn't mind seeing an explicit comparison to
non-zero for the fail case, but ack otherwise.
d
> The remove.c changes pretty much parallel the checks and logging done in
> add.c,
> if you're looking to see where the basic stuff from this patch came from.
>
> conf.c is the only user of rename() in the frontend, so this does add a new
> message there, but it seems like a worthwhile addition.
>
> -Dan
>
> lib/libalpm/dload.c | 7 +++++--
> lib/libalpm/remove.c | 24 ++++++++++++++++++------
> src/pacman/conf.c | 8 ++++++--
> 3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 23b8db7..dc4f91e 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -373,8 +373,11 @@ cleanup:
> }
>
> if(ret == 0) {
> - rename(tempfile, destfile);
> - if(final_file) {
> + if(rename(tempfile, destfile)) {
> + _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename
> %s to %s (%s)\n"),
> + tempfile, destfile, strerror(errno));
> + ret = -1;
> + } else if(final_file) {
> *final_file = strdup(strrchr(destfile, '/') + 1);
> }
> }
> diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
> index 83c437f..3c19c90 100644
> --- a/lib/libalpm/remove.c
> +++ b/lib/libalpm/remove.c
> @@ -220,7 +220,7 @@ static int can_remove_file(alpm_handle_t *handle, const
> alpm_file_t *file,
>
> /* Helper function for iterating through a package's file and deleting them
> * Used by _alpm_remove_commit. */
> -static void unlink_file(alpm_handle_t *handle, alpm_pkg_t *info,
> +static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *info,
> const alpm_file_t *fileobj, alpm_list_t *skip_remove, int
> nosave)
> {
> struct stat buf;
> @@ -234,7 +234,7 @@ static void unlink_file(alpm_handle_t *handle, alpm_pkg_t
> *info,
> if(alpm_list_find_str(skip_remove, fileobj->name)) {
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "%s is in skip_remove, skipping removal\n",
> file);
> - return;
> + return 1;
> }
>
> /* we want to do a lstat here, and not a _alpm_lstat.
> @@ -243,7 +243,7 @@ static void unlink_file(alpm_handle_t *handle, alpm_pkg_t
> *info,
> * actual symlink */
> if(lstat(file, &buf)) {
> _alpm_log(handle, ALPM_LOG_DEBUG, "file %s does not exist\n",
> file);
> - return;
> + return 1;
> }
>
> if(S_ISDIR(buf.st_mode)) {
> @@ -280,6 +280,7 @@ static void unlink_file(alpm_handle_t *handle, alpm_pkg_t
> *info,
> if(rmdir(file)) {
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "directory removal of
> %s failed: %s\n", file, strerror(errno));
> + return -1;
> } else {
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "removed directory %s
> (no remaining owners)\n", file);
> @@ -299,10 +300,16 @@ static void unlink_file(alpm_handle_t *handle,
> alpm_pkg_t *info,
> if(cmp != 0) {
> char newpath[PATH_MAX];
> snprintf(newpath, PATH_MAX,
> "%s.pacsave", file);
> - rename(file, newpath);
> + if(rename(file, newpath)) {
> + _alpm_log(handle,
> ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> + file, newpath,
> strerror(errno));
> + alpm_logaction(handle, "error:
> could not rename %s to %s (%s)\n",
> + file, newpath,
> strerror(errno));
> + return -1;
> + }
> _alpm_log(handle, ALPM_LOG_WARNING,
> _("%s saved as %s\n"), file, newpath);
> alpm_logaction(handle, "warning: %s
> saved as %s\n", file, newpath);
> - return;
> + return 0;
> }
> }
> }
> @@ -310,10 +317,14 @@ static void unlink_file(alpm_handle_t *handle,
> alpm_pkg_t *info,
> _alpm_log(handle, ALPM_LOG_DEBUG, "unlinking %s\n", file);
>
> if(unlink(file) == -1) {
> - _alpm_log(handle, ALPM_LOG_ERROR, _("cannot remove file
> '%s': %s\n"),
> + _alpm_log(handle, ALPM_LOG_ERROR, _("cannot remove %s
> (%s)\n"),
> file, strerror(errno));
> + alpm_logaction(handle, "error: cannot remove %s (%s)\n",
> + file, strerror(errno));
> + return -1;
> }
> }
> + return 0;
> }
>
> int _alpm_remove_single_package(alpm_handle_t *handle,
> @@ -397,6 +408,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle,
> for(i = filelist->count; i > 0; i--) {
> alpm_file_t *file = filelist->files + i - 1;
> int percent;
> + /* TODO: check return code and handle accordingly */
> unlink_file(handle, oldpkg, file, skip_remove,
> handle->trans->flags & ALPM_TRANS_FLAG_NOSAVE);
>
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index a580406..29e835c 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -184,10 +184,14 @@ static int download_with_xfercommand(const char *url,
> const char *localpath,
> ret = -1;
> } else {
> /* download was successful */
> + ret = 0;
> if(usepart) {
> - rename(tempfile, destfile);
> + if(rename(tempfile, destfile)) {
> + pm_printf(ALPM_LOG_ERROR, _("could not rename
> %s to %s (%s)\n"),
> + tempfile, destfile,
> strerror(errno));
> + ret = -1;
> + }
> }
> - ret = 0;
> }
>
> cleanup:
> --
> 1.7.6
>
>
