Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance documentation of zip_source_zip_file and zip_source_zip #433

Open
fdegros opened this issue May 14, 2024 · 2 comments
Open

Enhance documentation of zip_source_zip_file and zip_source_zip #433

fdegros opened this issue May 14, 2024 · 2 comments
Labels
documentation Documentation is incomplete, inaccurate, or unclear.

Comments

@fdegros
Copy link

fdegros commented May 14, 2024

The functions zip_source_zip_file and zip_source_zip take two parameters each of type zip_t* named archive and srcarchive.

zip_source_t* zip_source_zip_file(zip_t* archive, zip_t* srcarchive, zip_uint64_t srcidx, zip_flags_t flags, zip_uint64_t start, zip_int64_t length, const char* password);
zip_source_t* zip_source_zip(zip_t* archive, zip_t* srcarchive, zip_uint64_t srcidx, zip_flags_t flags, zip_uint64_t start, zip_int64_t len);

The role of the first archive parameter is not immediately obvious. Reading the documentation carefully, it seems that the archive parameter is used when setting the error status of a zip_t object in case of error. This raises a couple of questions:

  1. Is it possible for the archive and srcarchive pointers to point to the same zip_t object?
  2. Is it possible to pass a NULL pointer to the archive parameter? If yes, what happens in case of error? Is the error status then stored in *srcarchive or lost entirely?
  3. Would it make sense to rename the archive parameters to something more indicative of its role, like maybe error, since it is playing a similar role as the error parameter in zip_source_zip_file_create and zip_source_zip_create?

The role of the password parameter passed to zip_source_zip_file and zip_source_zip_file_create is not described in the documentation. Is it possible to pass a NULL pointer as password? If yes, does it have the same effect as passing an empty string?

@fdegros fdegros added the enhancement Request a new feature. label May 14, 2024
@dillof dillof added documentation Documentation is incomplete, inaccurate, or unclear. and removed enhancement Request a new feature. labels Jun 7, 2024
@dillof
Copy link
Member

dillof commented Jun 7, 2024

To answer your questions:

  1. Yes, that's no problem.
  2. No, passing NULL would crash. If you pass NULL as error to zip_source_zip_file_create(), the error information is lost.
  3. There are more zip_source_foo() / zip_source_foo_create() function pairs that all take archive or error as arguments, and keeping them consistent seems like a good idea. We'll add a paragraph to all their man pages to document these arguments.

If you pass NULL as password, the default password for the archive is used. If you pass the empty string, that is used as password.

@fdegros
Copy link
Author

fdegros commented Jun 10, 2024

If you pass NULL as password, the default password for the archive is used. If you pass the empty string, that is used as password.

This might be worth documenting very clearly. In particular, this is a place where a NULL string and an empty string have different meanings and effects. This can be quite tricky.

This is also subtly different from the way zip_set_default_password treats its password parameter. See zip_set_default_password's documentation:

If password is NULL or the empty string, the default password is unset.

In that case, a NULL or an empty string have the same meaning and effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation is incomplete, inaccurate, or unclear.
Projects
None yet
Development

No branches or pull requests

2 participants