On Fri, Oct 23, 2009 at 3:54 PM, Laszlo Papp <djszapi2@gmail.com> wrote:
Fix some warning that cppcheck gave back
* opendir(path)) == (DIR *)-1 is maybe the result of miss understanding of the man page, if the opendir wasn't successful it gives back NULL instead of '(DIR *)-1'.
* return(PM_ERR_NOT_A_DIR); was established instead of hard coding numbers for return statement.
* The ambiguity while cycle with EINTR condition was refactored for a do {} while () cycle to be easier to read/understand
Signed-off-by: Laszlo Papp <djszapi@archlinux.us> ---
lib/libalpm/util.c | 11 ++++++----- src/pacman/util.c | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index d910809..ad3dc2c 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -212,8 +212,9 @@ int _alpm_lckmk() _alpm_makepath(dir); FREE(dir);
- while((fd = open(file, O_WRONLY | O_CREAT | O_EXCL, 0000)) == -1 - && errno == EINTR); + do { + fd = open(file, O_WRONLY | O_CREAT | O_EXCL, 0000); + } while(fd == -1 && errno == EINTR); if(fd > 0) { FILE *f = fdopen(fd, "w"); fprintf(f, "%ld\n", (long)getpid()); @@ -315,7 +316,7 @@ int _alpm_unpack(const char *archive, const char *prefix, alpm_list_t *list, int
st = archive_entry_stat(entry); entryname = archive_entry_pathname(entry); - + if(S_ISREG(st->st_mode)) { archive_entry_set_perm(entry, 0644); } else if(S_ISDIR(st->st_mode)) { @@ -389,8 +390,8 @@ int _alpm_rmrf(const char *path) } } } else { - if((dirp = opendir(path)) == (DIR *)-1) { - return(1); + if((dirp = opendir(path)) == NULL) { + return(PM_ERR_NOT_A_DIR); This is not how these error codes are intended to be used; instead
Cool, these are good catches. Just a few suggestions. 1. Stop tabbing your commit message 2. do/while is a bit more understandable; can you fix the other two places we use that construct as well? (lib/libalpm/trans.c, src/pacman/util.c, one more in lib/libalpm/util.c) pmerrno_t should be set. However, since this is an internal function only, we shouldn't be doing this (it should be handled only in a exposed function that would fail if this particular call failed. Take a look at the RET_ERR macro for an example.
} for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { if(dp->d_ino) { diff --git a/src/pacman/util.c b/src/pacman/util.c index 1143bef..e3dbfbb 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -122,8 +122,8 @@ int rmrf(const char *path) return(1); }
- if((dirp = opendir(path)) == (DIR *)-1) { - return(1); + if((dirp = opendir(path)) == NULL) { + return(PM_ERR_NOT_A_DIR); This doesn't make a whole lot of sense in the frontend.
} for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { if(dp->d_ino) { -- 1.6.5