Created attachment 275753 [details] [review]
archive: Ignore filenames consisting of a single "."

Don't abort if a path has a component consisting of a single ".",
just ignore that component.
E.g. OpenCV-2.3.1/./opencv.pc.cmake.in becomes
OpenCV-2.3.1/opencv.pc.cmake.in

Review of attachment 275753 [details] [review]:

It looks good, thanks!

Pushed to master as 46bdbf1d4596 and gnome-3-12 as 58a78a454a9c. Thanks for the review.

Created attachment 284393 [details] [review]
archive: Allow reading files with '/./' in the path

Allow reading files with a "." component in the path.  This is a
followup to 46bdbf1d4596 ('archive: Ignore filenames consisting of a
single "."'), which allowed the archive backend to enumerate an archive
with a single "." in the path.

Created attachment 284394 [details] [review]
archive: Fix some memory leaks

Unfortunately, the clever trick to avoid string copying causes leaks
because it inserts NULLs in the middle of a NULL-terminated array.

Review of attachment 284393 [details] [review]:

Wouldn't be better to create new function to fix the path and use it there and also in archive_file_get_from_path?

Created attachment 284923 [details] [review]
archive: Fix some memory leaks

Unfortunately, the clever trick to avoid string copying causes leaks
because it inserts NULLs in the middle of a NULL-terminated array.

Created attachment 284924 [details] [review]
archive: Allow reading files with '/./' in the path

Allow reading files with a "." component in the path.  This is a
followup to 46bdbf1d4596 ('archive: Ignore filenames consisting of a
single "."'), which allowed the archive backend to enumerate an archive
with a single "." in the path.

Implement this with a generic function to fixup an archive_entry path,
and use this wherever archive_entry_pathname() is used.

(In reply to comment #6)
> Review of attachment 284393 [details] [review]:
> 
> Wouldn't be better to create new function to fix the path and use it there and
> also in archive_file_get_from_path?

Thanks for the review. I implemented a function to fix the path and used it wherever archive_entry_pathname() is used since those are the paths that need fixing (archive_file_get_from_path() is called with gvfs paths in the lookup case which is why I don't call the fixup path function there).

Review of attachment 284923 [details] [review]:

The patch looks correct and the code is more readable now, however I don't see possibility of memory leak with the current solution. It is right NULLs are inserted in the middle, but they are inserted from the middle to the end of the array. So g_strfreev releases strings from the start to the middle of array (and the rest of the strings is used or freed in the loop before). Or am I wrong?

Review of attachment 284924 [details] [review]:

Thanks for that patch, it looks good besides few notes:

::: daemon/gvfsbackendarchive.c
@@ +290,3 @@
+  /* strip trailing slash from the path */
+  len = strlen (str);
+  if (len > 0 && str[len - 1] == '/')

Wouldn't be better to use g_str_has_suffix instead?

@@ +763,3 @@
           if (g_str_equal (entry_pathname, filename + 1))
             {
+              g_free(entry_pathname);

Please add a space between the command and parenthesis.

@@ +774,3 @@
             archive_read_data_skip (archive->archive);
+
+          g_free(entry_pathname);

dtto

(In reply to comment #10)
> Review of attachment 284923 [details] [review]:
> 
> The patch looks correct and the code is more readable now, however I don't see
> possibility of memory leak with the current solution. It is right NULLs are
> inserted in the middle, but they are inserted from the middle to the end of the
> array. So g_strfreev releases strings from the start to the middle of array
> (and the rest of the strings is used or freed in the loop before). Or am I
> wrong?

Well it probably worked OK as it was originally written, but then I changed it to work with paths like "a/b/./c/d "
In this case, the array becomes "a, b, NULL, c, d" and c and d are leaked.

(In reply to comment #11)
> Review of attachment 284924 [details] [review]:
> 
> Thanks for that patch, it looks good besides few notes:
> 
> ::: daemon/gvfsbackendarchive.c
> @@ +290,3 @@
> +  /* strip trailing slash from the path */
> +  len = strlen (str);
> +  if (len > 0 && str[len - 1] == '/')
> 
> Wouldn't be better to use g_str_has_suffix instead?
> 

Since we need the length to remove the trailing slash, doing it this way saves an extra iteration through the entire string (which g_str_has_suffix would have to do since it doesn't know the length of the string).

You are right, so please commit both patches (just only with those additional spaces). Thanks!

Pushed to master as ce9d7700c5641610d1c4c2400c2ab3f9183288fb (with additional spaces).  Thanks for the review!