Re: [pacman-dev] [PATCH] libalpm: fix documentation for alpm_pkg_mtree_next

20 Jun 2019

On 14/6/19 11:26 pm, Dave Reisner wrote:
...
On Fri, Jun 14, 2019 at 02:19:52PM +0100, Morgan Adamiec wrote:
...
On Fri, 14 Jun 2019 at 14:09, Dave Reisner  wrote:
...
On Fri, Jun 14, 2019 at 02:51:14AM +0100, morganamilo wrote:
...
libarchive uses 1 for EOF, not 0. Instead of using the actual ints, use
libarchive's error codes.
---
By the way, not familiar with doxygen. Is my wording fine or is there
some built in "see also" functionality?

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index ffb2ad96..ece894cf 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -1326,7 +1326,8 @@ struct archive *alpm_pkg_mtree_open(alpm_pkg_t *pkg);
  * @param pkg the package that the mtree file is being read from
  * @param archive the archive structure reading from the mtree file
  * @param entry an archive_entry to store the entry header information
- * @return 0 if end of archive is reached, non-zero otherwise.
+ * @return ARCHIVE_OK on success, ARCHIVE_EOF if end of archive is reached,
+ * otherwise an error occured (see archive.h).
Please, no. Let's not leak details from libarchive in our own API.
...
*/
 int alpm_pkg_mtree_next(const alpm_pkg_t *pkg, struct archive *archive,
              struct archive_entry **entry);
--
2.21.0
Why not? The return value is exactly that. If libarchive's return
codes suddenly changed then so would libalpms's. Plus pacman itself
already uses ARCHIVE_OK to check the return code. And finally if we
did not depend on magic numbers then the doc wouldn't be wrong in the
first place.
Because users of libalpm should only need to understand libalpm and not
concern themselves with details of libarchive. Exposing ARCHIVE_* in
libalpm is a leaky abstraction.
If the code is broken (and it sounds like it is), then it should be
fixed along with the documentation.
Agreed.   Not this is the only place in pacman we use an ARCHIVE_*
value, so this is broken.

src/pacman/check.c:     while(alpm_pkg_mtree_next(pkg, mtree, &entry) ==
ARCHIVE_OK) {

    

Re: [pacman-dev] [PATCH] libalpm: fix documentation for alpm_pkg_mtree_next

Allan McRae