Skip to content

Commit

Permalink
fix(core): no need to call zip_discard, it's handled in zip_close
Browse files Browse the repository at this point in the history
  • Loading branch information
@extrafu
extrafu committed Jun 19, 2020
1 parent 67f5e5e commit 1389dcf
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion SoObjects/SOGo/SOGoZipArchiver.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ - (BOOL)close
if (self->z != NULL) {
if (zip_close(self->z) != 0) {
NSLog(@"Failed to close zip archive: %@", [NSString stringWithCString: zip_strerror(self->z)]);
zip_discard(self->z);
success = NO;
}
self->z = NULL;
Expand Down

4 comments on commit 1389dcf

@jkanefendt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about that? My interpretation from the man-page is that the zip handle needs to be discarded explicetely if zip_close failed:

The zip_close() function writes any changes made to archive to disk. If archive contains no files, the file is completely removed (no empty archive is written). If successful, archive is freed. Otherwise archive is left unchanged and must still be freed.
To close and free a zip archive without saving changes, use zip_discard(3).

@extrafu
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case we need to find how to detect if zip_discard is present at compilation time. There are at least 3 versions of libzip to support and zip_discard was added in libzip 0.11 and there is no way to detect that version.

@jkanefendt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at /usr/include/x86_64-linux-gnu/zipconf.h (the actual path might depend on the distro/arch) which is included from zip.h:

#define LIBZIP_VERSION_MAJOR 1
#define LIBZIP_VERSION_MINOR 1
#define LIBZIP_VERSION_MICRO 2

So this should work for zip_discard as well as for other compat issues (e.g. LIBZIP_VERSION_MAJOR >= 1 for zip_error_init_with_code):

#if defined(LIBZIP_VERSION_MAJOR) && (LIBZIP_VERSION_MAJOR > 0 || VERSION_MINOR >= 11)
            zip_discard(self->z);
#endif

@extrafu
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know about that file and those variables. I overlooked the fact they were defined in 0.11 as they weren't in 0.10 and 0.9.

Do a small PR and I'll merge it.

Please sign in to comment.