Re: [pacman-dev] [PATCH] pacman: print error when -Fx is given invalid regex

[Allan McRae]

On 12/11/19 11:48 am, morganamilo wrote:
> When processing the targets for -Fx, compile all the regex ahead of
> time, printing an error for each that failed to compile. Then, if they all
> compiled successfully, continue with printing files.
> 
> Signed-off-by: morganamilo 
> 
> diff --git a/src/pacman/files.c b/src/pacman/files.c
> index 3b6dc23b..91d891a6 100644
> --- a/src/pacman/files.c
> +++ b/src/pacman/files.c
> @@ -94,17 +94,26 @@ static void print_match(alpm_list_t *match, alpm_db_t 
> *repo, alpm_pkg_t *pkg, in
>   }
>  }
>  
> +struct filetarget {
> + char *targ;
> + int exact_file;
> + regex_t reg;
> +};
> +
> +static void filetarget_free(struct filetarget *ftarg) {
> + regfree(>reg);
> + free(ftarg);
> +}
> +


Rest of the patch looks fine.  But first thing I saw here was we are not
freeing char* targ.  But we don't have to because the memory is held
elsewhere.

Add a comment to clarify that an I am happy with the patch.


Also:

.git/rebase-apply/patch:42: trailing whitespace.
pm_printf(ALPM_LOG_ERROR,
warning: 1 line adds whitespace errors.


You can enable a git hook to stop you committing files with whitespace
issues.


As an aside, now we precompile all regex, I wonder if we should loop
though them withing each package?  e.g. "pacman -F foo bar" in Arch
Luinux gives a result for community/phonegap in two different places in
the output.

A

[pacman-dev] [PATCH] pacman: print error when -Fx is given invalid regex

[morganamilo]

When processing the targets for -Fx, compile all the regex ahead of
time, printing an error for each that failed to compile. Then, if they all
compiled successfully, continue with printing files.

Signed-off-by: morganamilo 

diff --git a/src/pacman/files.c b/src/pacman/files.c
index 3b6dc23b..91d891a6 100644
--- a/src/pacman/files.c
+++ b/src/pacman/files.c
@@ -94,17 +94,26 @@ static void print_match(alpm_list_t *match, alpm_db_t 
*repo, alpm_pkg_t *pkg, in
}
 }
 
+struct filetarget {
+   char *targ;
+   int exact_file;
+   regex_t reg;
+};
+
+static void filetarget_free(struct filetarget *ftarg) {
+   regfree(>reg);
+   free(ftarg);
+}
+
 static int files_search(alpm_list_t *syncs, alpm_list_t *targets, int regex) {
int ret = 0;
-   alpm_list_t *t;
+   alpm_list_t *t, *filetargs = NULL;
 
for(t = targets; t; t = alpm_list_next(t)) {
char *targ = t->data;
-   alpm_list_t *s;
-   int found = 0;
-   regex_t reg;
size_t len = strlen(targ);
int exact_file = strchr(targ, '/') != NULL;
+   regex_t reg;
 
if(exact_file) {
while(len > 1 && targ[0] == '/') {
@@ -115,11 +124,33 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
 
if(regex) {
if(regcomp(, targ, REG_EXTENDED | REG_NOSUB | 
REG_ICASE | REG_NEWLINE) != 0) {
-   /* TODO: error message */
-   goto notfound;
+   pm_printf(ALPM_LOG_ERROR, 
+   _("invalid regular expression 
'%s'\n"), targ);
+   ret = 1;
+   continue;
}
}
 
+   struct filetarget *ftarg = malloc(sizeof(struct filetarget));
+   ftarg->targ = targ;
+   ftarg->exact_file = exact_file;
+   ftarg->reg = reg;
+
+   filetargs = alpm_list_add(filetargs, ftarg);
+   }
+
+   if(ret != 0) {
+   goto cleanup;
+   }
+
+   for(t = filetargs; t; t = alpm_list_next(t)) {
+   struct filetarget *ftarg = t->data;
+   char *targ = ftarg->targ;
+   regex_t *reg = >reg;
+   int exact_file = ftarg->exact_file;
+   alpm_list_t *s;
+   int found = 0;
+
for(s = syncs; s; s = alpm_list_next(s)) {
alpm_list_t *p;
alpm_db_t *repo = s->data;
@@ -135,7 +166,7 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
if (regex) {
for(size_t f = 0; f < 
files->count; f++) {
char *c = 
files->files[f].name;
-   if(regexec(, c, 0, 
0, 0) == 0) {
+   if(regexec(reg, c, 0, 
0, 0) == 0) {
match = 
alpm_list_add(match, files->files[f].name);
found = 1;
}
@@ -151,7 +182,7 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
char *c = 
strrchr(files->files[f].name, '/');
if(c && *(c + 1)) {
if(regex) {
-   m = 
regexec(, (c + 1), 0, 0, 0);
+   m = 
regexec(reg, (c + 1), 0, 0, 0);
} else {
m = strcmp(c + 
1, targ);
}
@@ -170,16 +201,15 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
}
}
 
-   if(regex) {
-   regfree();
-   }
-
-notfound:
if(!found) {
ret = 1;
}
}
 
+cleanup:
+   alpm_list_free_inner(filetargs, (alpm_list_fn_free) filetarget_free);
+   alpm_list_free(filetargs);
+
return ret;
 }
 
-- 
2.23.0