On Fri, Jun 14, 2019 at 02:19:52PM +0100, Morgan Adamiec wrote: > On Fri, 14 Jun 2019 at 14:09, Dave Reisner <d...@falconindy.com> 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.