[pacman-dev] [PATCH] Refactor do-while cycle in the api, util.c
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/util.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index cf2d623..f54b2bf 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)) { -- 1.6.5.1
On Wed, Nov 11, 2009 at 3:08 PM, Laszlo Papp <djszapi@archlinux.us> wrote:
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/util.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
I have no idea why it doesn't apply cleanly, but I'm not going to bust my butt at 2am working on this. Can you grab the raw email and apply it yourself to a clean master? Most likely it is the whitespace change you tried to sneak in there, as I don't see any whitespace after the "-" in the actual email body. If you resubmit without that this will get applied. And argh, now I'm rambling, but why didn't we fix the ones in trans.c, the other one in util.c, pacman.c, etc? -Dan dmcgee@galway ~/projects/pacman (master) $ git am -3 -s < /tmp/do-while.patch Applying: Refactor do-while cycle in the api, util.c Using index info to reconstruct a base tree... error: patch failed: lib/libalpm/util.c:315 error: lib/libalpm/util.c: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge. Patch failed at 0001 Refactor do-while cycle in the api, util.c When you have resolved this problem run "git am -3 --resolved". If you would prefer to skip this patch, instead run "git am -3 --skip". To restore the original branch and stop patching run "git am -3 --abort".
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/trans.c | 4 +++- lib/libalpm/util.c | 11 +++++++---- src/pacman/pacman.c | 4 +++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index aea71db..bc594f9 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -261,7 +261,9 @@ int SYMEXPORT alpm_trans_release() /* unlock db */ if(!nolock_flag) { if(handle->lckfd != -1) { - while(close(handle->lckfd) == -1 && errno == EINTR); + int fd; + fd = close(handle->lckfd); + while( fd == -1 && errno == EINTR); handle->lckfd = -1; } if(_alpm_lckrm()) { diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index cf2d623..005e004 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)) { @@ -511,7 +512,9 @@ int _alpm_run_chroot(const char *root, const char *cmd) /* this code runs for the parent only (wait on the child) */ pid_t retpid; int status; - while((retpid = waitpid(pid, &status, 0)) == -1 && errno == EINTR); + do { + retpid = waitpid(pid, &status, 0); + } while( retpid == -1 && errno == EINTR); if(retpid == -1) { _alpm_log(PM_LOG_ERROR, _("call to waitpid failed (%s)\n"), strerror(errno)); diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f4f8044..eb01824 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -234,7 +234,9 @@ static void cleanup(int ret) { static ssize_t xwrite(int fd, const void *buf, size_t count) { ssize_t ret; - while((ret = write(fd, buf, count)) == -1 && errno == EINTR); + do { + ret = write(fd, buf, count); + } while( ret == -1 && errno == EINTR); return(ret); } -- 1.6.5
On Fri, Nov 13, 2009 at 12:10 AM, Laszlo Papp <djszapi2@gmail.com> wrote:
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/trans.c | 4 +++- lib/libalpm/util.c | 11 +++++++---- src/pacman/pacman.c | 4 +++- 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index aea71db..bc594f9 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -261,7 +261,9 @@ int SYMEXPORT alpm_trans_release() /* unlock db */ if(!nolock_flag) { if(handle->lckfd != -1) { - while(close(handle->lckfd) == -1 && errno == EINTR); + int fd; + fd = close(handle->lckfd); + while( fd == -1 && errno == EINTR); handle->lckfd = -1; }
Hmmmmmmmmmmm ok. Thanks ? Bye ?
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/trans.c | 5 ++++- lib/libalpm/util.c | 11 +++++++---- src/pacman/pacman.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index aea71db..17fc1b3 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -261,7 +261,10 @@ int SYMEXPORT alpm_trans_release() /* unlock db */ if(!nolock_flag) { if(handle->lckfd != -1) { - while(close(handle->lckfd) == -1 && errno == EINTR); + int fd; + do { + fd = close(handle->lckfd); + } while( fd == -1 && errno == EINTR); handle->lckfd = -1; } if(_alpm_lckrm()) { diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index cf2d623..005e004 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)) { @@ -511,7 +512,9 @@ int _alpm_run_chroot(const char *root, const char *cmd) /* this code runs for the parent only (wait on the child) */ pid_t retpid; int status; - while((retpid = waitpid(pid, &status, 0)) == -1 && errno == EINTR); + do { + retpid = waitpid(pid, &status, 0); + } while( retpid == -1 && errno == EINTR); if(retpid == -1) { _alpm_log(PM_LOG_ERROR, _("call to waitpid failed (%s)\n"), strerror(errno)); diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f4f8044..eb01824 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -234,7 +234,9 @@ static void cleanup(int ret) { static ssize_t xwrite(int fd, const void *buf, size_t count) { ssize_t ret; - while((ret = write(fd, buf, count)) == -1 && errno == EINTR); + do { + ret = write(fd, buf, count); + } while( ret == -1 && errno == EINTR); return(ret); } -- 1.6.5
On Fri, Nov 13, 2009 at 12:59 AM, Laszlo Papp <djszapi2@gmail.com> wrote:
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/trans.c | 5 ++++- lib/libalpm/util.c | 11 +++++++---- src/pacman/pacman.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-)
The 6th iteration of this trivial patch looks good to me. Thanks for your hard work :)
On Fri, Nov 13, 2009 at 1:15 AM, Xavier <shiningxc@gmail.com> wrote:
On Fri, Nov 13, 2009 at 12:59 AM, Laszlo Papp <djszapi2@gmail.com> wrote:
* It makes the code clearer to read/understand * Cppcheck tool doesn't show this anymore: [./util.c:215]: (error) Resource leak: fd --- lib/libalpm/trans.c | 5 ++++- lib/libalpm/util.c | 11 +++++++---- src/pacman/pacman.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-)
The 6th iteration of this trivial patch looks good to me.
Thanks for your hard work :)
Okay, Dan applied it, thank you the helps Xavier, really. Best Regards, Laszlo Papp
participants (4)
-
Dan McGee
-
Laszlo Papp
-
Laszlo Papp
-
Xavier