[pacman-dev] [PATCH] query_fileowner: break/continue pkgfile loop
Break out of pkgfile loop on match or continue if the pkgfile path is too long. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pacman/query.c b/src/pacman/query.c index 9b1ea6f..a14b3e3 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -212,12 +212,14 @@ static int query_fileowner(alpm_list_t *targets) if(strcmp(pkgfile, bname) == 0) { print_query_fileowner(filename, info); found = 1; + break; } continue; } if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); + continue; } /* concatenate our file and the root path */ strcpy(path + rootlen, pkgfile); @@ -229,6 +231,8 @@ static int query_fileowner(alpm_list_t *targets) if(ppath && strcmp(ppath, rpath) == 0) { print_query_fileowner(filename, info); found = 1; + free(ppath); + break; } free(ppath); } -- 1.7.11.1
On 08/07/12 05:12, Andrew Gregory wrote:
Break out of pkgfile loop on match or continue if the pkgfile path is too long.
I agree with the continue - it is stupid we check the length and then do nothing when it fails.... However, I do not agree with the two breaks. The breaks make the whole "found = 1" redundant. And that is useful for directory ownership querying with a loop using "(!found || isdir)", which all -Qo <dir> patches have used so far. You can either ping a patch with the just continue or let me know and I will make one to push. Allam
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/query.c b/src/pacman/query.c index 9b1ea6f..a14b3e3 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -212,12 +212,14 @@ static int query_fileowner(alpm_list_t *targets) if(strcmp(pkgfile, bname) == 0) { print_query_fileowner(filename, info); found = 1; + break; } continue; }
if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); + continue; } /* concatenate our file and the root path */ strcpy(path + rootlen, pkgfile); @@ -229,6 +231,8 @@ static int query_fileowner(alpm_list_t *targets) if(ppath && strcmp(ppath, rpath) == 0) { print_query_fileowner(filename, info); found = 1; + free(ppath); + break; } free(ppath); }
On Sun, 15 Jul 2012 19:23:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 08/07/12 05:12, Andrew Gregory wrote:
Break out of pkgfile loop on match or continue if the pkgfile path is too long.
I agree with the continue - it is stupid we check the length and then do nothing when it fails....
However, I do not agree with the two breaks. The breaks make the whole "found = 1" redundant. And that is useful for directory ownership querying with a loop using "(!found || isdir)", which all -Qo <dir> patches have used so far.
You can either ping a patch with the just continue or let me know and I will make one to push.
Allam
The breaks are for the *file* loop. "found = 1" is for exiting the *package* loop. Breaking the file loop is proper when checking directory ownership as well. Right now, when pacman finds a match it continues checking the rest of the current package's files. In fact, the reason I used break for this instead of adding a !found check in the file loop is because that wouldn't be as friendly to the directory ownership patches. Andrew
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/query.c b/src/pacman/query.c index 9b1ea6f..a14b3e3 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -212,12 +212,14 @@ static int query_fileowner(alpm_list_t *targets) if(strcmp(pkgfile, bname) == 0) { print_query_fileowner(filename, info); found = 1; + break; } continue; }
if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); + continue; } /* concatenate our file and the root path */ strcpy(path + rootlen, pkgfile); @@ -229,6 +231,8 @@ static int query_fileowner(alpm_list_t *targets) if(ppath && strcmp(ppath, rpath) == 0) { print_query_fileowner(filename, info); found = 1; + free(ppath); + break; } free(ppath); }
On 15/07/12 23:08, Andrew Gregory wrote:
On Sun, 15 Jul 2012 19:23:22 +1000 Allan McRae <allan@archlinux.org> wrote:
On 08/07/12 05:12, Andrew Gregory wrote:
Break out of pkgfile loop on match or continue if the pkgfile path is too long.
I agree with the continue - it is stupid we check the length and then do nothing when it fails....
However, I do not agree with the two breaks. The breaks make the whole "found = 1" redundant. And that is useful for directory ownership querying with a loop using "(!found || isdir)", which all -Qo <dir> patches have used so far.
You can either ping a patch with the just continue or let me know and I will make one to push.
Allam
The breaks are for the *file* loop. "found = 1" is for exiting the *package* loop. Breaking the file loop is proper when checking directory ownership as well. Right now, when pacman finds a match it continues checking the rest of the current package's files. In fact, the reason I used break for this instead of adding a !found check in the file loop is because that wouldn't be as friendly to the directory ownership patches.
Andrew
Ah crap... going for a fast review of all my pending patches and obviously went too fast. Patch looks fine. Will pull to my working branch. Allan
participants (2)
-
Allan McRae
-
Andrew Gregory