Re: [pacman-dev] [PATCH v3 3/4] Declare alpm_pkg_get_(origin|reason) to return int

[Andrew Gregory]

On 10/17/16 at 10:17pm, Allan McRae wrote:
> On 13/10/16 06:13, ivy.fos...@gmail.com wrote:
> > From: Ivy Foster 
> > 
> > These functions can return -1 on error, which is not included in the
> > enumerated types they were declared to return.
> > 
> > Signed-off-by: Ivy Foster 
> > ---
> 
> 
> be_local.c: In function ‘alpm_pkg_set_reason’:
> be_local.c:1124:30: error: comparison between signed and unsigned
> integer expressions [-Werror=sign-compare]
>   if(alpm_pkg_get_reason(pkg) == reason) {
>   ^~
> 
> You can use --enable-warningflags to build with many of our current
> warning flags enabled.
> 
> Why change the return to an int and not add an UNKNOWN value to the enum
> (with value 0 or 1 << 30)?
> 
> Allan

The only way get_reason and get_origin return -1 is if pkg is NULL, so
UNKNOWN isn't really accurate.  We could add an ERROR value, but I was
hoping to avoid that.

apg

[pacman-dev] [PATCH v3 3/4] Declare alpm_pkg_get_(origin|reason) to return int

[ivy . foster]

From: Ivy Foster 

These functions can return -1 on error, which is not included in the
enumerated types they were declared to return.

Signed-off-by: Ivy Foster 
---
 lib/libalpm/alpm.h| 6 +++---
 lib/libalpm/package.c | 4 ++--
 src/pacman/package.c  | 3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 2d2491d..6ad3a08 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -1166,7 +1166,7 @@ const char *alpm_pkg_get_version(alpm_pkg_t *pkg);
 /** Returns the origin of the package.
  * @return an alpm_pkgfrom_t constant, -1 on error
  */
-alpm_pkgfrom_t alpm_pkg_get_origin(alpm_pkg_t *pkg);
+int alpm_pkg_get_origin(alpm_pkg_t *pkg);
 
 /** Returns the package description.
  * @param pkg a pointer to package
@@ -1233,9 +1233,9 @@ off_t alpm_pkg_get_isize(alpm_pkg_t *pkg);
 
 /** Returns the package installation reason.
  * @param pkg a pointer to package
- * @return an enum member giving the install reason.
+ * @return an enum member giving the install reason, or -1 on error.
  */
-alpm_pkgreason_t alpm_pkg_get_reason(alpm_pkg_t *pkg);
+int alpm_pkg_get_reason(alpm_pkg_t *pkg);
 
 /** Returns the list of package licenses.
  * @param pkg a pointer to package
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 05becf0..601625e 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -209,7 +209,7 @@ const char SYMEXPORT *alpm_pkg_get_version(alpm_pkg_t *pkg)
return pkg->version;
 }
 
-alpm_pkgfrom_t SYMEXPORT alpm_pkg_get_origin(alpm_pkg_t *pkg)
+int SYMEXPORT alpm_pkg_get_origin(alpm_pkg_t *pkg)
 {
ASSERT(pkg != NULL, return -1);
pkg->handle->pm_errno = ALPM_ERR_OK;
@@ -293,7 +293,7 @@ off_t SYMEXPORT alpm_pkg_get_isize(alpm_pkg_t *pkg)
return pkg->ops->get_isize(pkg);
 }
 
-alpm_pkgreason_t SYMEXPORT alpm_pkg_get_reason(alpm_pkg_t *pkg)
+int SYMEXPORT alpm_pkg_get_reason(alpm_pkg_t *pkg)
 {
ASSERT(pkg != NULL, return -1);
pkg->handle->pm_errno = ALPM_ERR_OK;
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 62ed7ce..9be2415 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -193,7 +193,8 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
 {
unsigned short cols;
time_t bdate, idate;
-   alpm_pkgfrom_t from;
+   /* Either an alpm_pkgfrom_t, or -1 on error */
+   int from;
double size;
char bdatestr[50] = "", idatestr[50] = "";
const char *label, *reason;
-- 
2.10.0