[pacman-dev] [PATCH] Constify some input pointers

[Rikard Falkeborn]

Signed-off-by: Rikard Falkeborn 
---
 lib/libalpm/diskspace.c | 2 +-
 lib/libalpm/diskspace.h | 2 +-
 lib/libalpm/util.c  | 2 +-
 src/pacman/util.c   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c
index 4cc17a12..2be98055 100644
--- a/lib/libalpm/diskspace.c
+++ b/lib/libalpm/diskspace.c
@@ -359,7 +359,7 @@ static int check_mountpoint(alpm_handle_t *handle, 
alpm_mountpoint_t *mp)
 }
 
 int _alpm_check_downloadspace(alpm_handle_t *handle, const char *cachedir,
-   size_t num_files, off_t *file_sizes)
+   size_t num_files, const off_t *file_sizes)
 {
alpm_list_t *mount_points;
alpm_mountpoint_t *cachedir_mp;
diff --git a/lib/libalpm/diskspace.h b/lib/libalpm/diskspace.h
index 4f7d00b9..5f17a878 100644
--- a/lib/libalpm/diskspace.h
+++ b/lib/libalpm/diskspace.h
@@ -58,6 +58,6 @@ typedef struct __alpm_mountpoint_t {
 
 int _alpm_check_diskspace(alpm_handle_t *handle);
 int _alpm_check_downloadspace(alpm_handle_t *handle, const char *cachedir,
-   size_t num_files, off_t *file_sizes);
+   size_t num_files, const off_t *file_sizes);
 
 #endif /* ALPM_DISKSPACE_H */
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index cb838e43..ead03004 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -1006,7 +1006,7 @@ static int sha256_file(const char *path, unsigned char 
output[32])
  * @return a NULL terminated string with the hexadecimal representation of
  * bytes or NULL on error. This string must be freed.
  */
-static char *hex_representation(unsigned char *bytes, size_t size)
+static char *hex_representation(const unsigned char *bytes, size_t size)
 {
static const char *hex_digits = "0123456789abcdef";
char *str;
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 6693ec75..97b8e06d 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -515,7 +515,7 @@ void string_display(const char *title, const char *string, 
unsigned short cols)
 }
 
 static void table_print_line(const alpm_list_t *line, short col_padding,
-   size_t colcount, size_t *widths, int *has_data)
+   size_t colcount, const size_t *widths, const int *has_data)
 {
size_t i;
int need_padding = 0;
-- 
2.26.2

[pacman-dev] [PATCH] Add REALLOC macro to simplify realloc error handling

[Rikard Falkeborn]

realloc can fail just like the other memory allocation functions. Add a
macro to simplify handling of realloc failures, similar to the already
existing MALLOC, CALLOC, etc.

Replace the existing realloc uses with the new macro, allowing us to
move tedious error handling to the macro. Also, in be_package and
be_sync, this fixes hypothetical memory leaks (and thereafter null
pointer dereferences) in case realloc fails to shrink the allocated
memory.

Signed-off-by: Rikard Falkeborn 
---
 lib/libalpm/be_local.c   |  7 +--
 lib/libalpm/be_package.c |  3 +--
 lib/libalpm/be_sync.c|  2 +-
 lib/libalpm/util.c   | 13 +++--
 lib/libalpm/util.h   |  1 +
 5 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 9ebdfa40..689f0102 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -843,12 +843,7 @@ static int local_db_read(alpm_pkg_t *info, int inforeq)
}
/* attempt to hand back any memory we don't 
need */
if(files_count > 0) {
-   alpm_file_t *newfiles;
-
-   newfiles = realloc(files, 
sizeof(alpm_file_t) * files_count);
-   if(newfiles != NULL) {
-   files = newfiles;
-   }
+   REALLOC(files, sizeof(alpm_file_t) * 
files_count, (void)0);
} else {
FREE(files);
}
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 7a118d2a..4832966b 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -669,8 +669,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle,
if(full) {
if(newpkg->files.files) {
/* attempt to hand back any memory we don't need */
-   newpkg->files.files = realloc(newpkg->files.files,
-   sizeof(alpm_file_t) * 
newpkg->files.count);
+   REALLOC(newpkg->files.files, sizeof(alpm_file_t) * 
newpkg->files.count, (void)0);
/* "checking for conflicts" requires a sorted list, 
ensure that here */
_alpm_log(handle, ALPM_LOG_DEBUG,
"sorting package filelist for %s\n", 
pkgfile);
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index aafed15d..5f457122 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -689,7 +689,7 @@ static int sync_db_read(alpm_db_t *db, struct archive 
*archive,
}
/* attempt to hand back any memory we don't 
need */
if(files_count > 0) {
-   files = realloc(files, 
sizeof(alpm_file_t) * files_count);
+   REALLOC(files, sizeof(alpm_file_t) * 
files_count, (void)0);
} else {
FREE(files);
}
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index c46b1397..cebc87ec 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -1427,22 +1427,15 @@ int _alpm_fnmatch(const void *pattern, const void 
*string)
  */
 void *_alpm_realloc(void **data, size_t *current, const size_t required)
 {
-   char *newdata;
-
-   newdata = realloc(*data, required);
-   if(!newdata) {
-   _alpm_alloc_fail(required);
-   return NULL;
-   }
+   REALLOC(*data, required, return NULL);
 
if (*current < required) {
/* ensure all new memory is zeroed out, in both the initial
 * allocation and later reallocs */
-   memset(newdata + *current, 0, required - *current);
+   memset((char*)*data + *current, 0, required - *current);
}
*current = required;
-   *data = newdata;
-   return newdata;
+   return *data;
 }
 
 /** This automatically grows data based on current/required.
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
index 675eedec..3306a022 100644
--- a/lib/libalpm/util.h
+++ b/lib/libalpm/util.h
@@ -53,6 +53,7 @@ void _alpm_alloc_fail(size_t size);
 
 #define MALLOC(p, s, action) do { p = malloc(s); if(p == NULL) { 
_alpm_alloc_fail(s); action; } } while(0)
 #define CALLOC(p, l, s, action) do { p = calloc(l, s); if(p == NULL) { 
_alpm_alloc_fail(l * s); action; } } while(0)
+#define REALLOC(p, s, action) do { void* np = realloc(p, s); if(np == NULL) { 
_alpm_alloc_fail(s); action; } else { p = np; } } while(0)
 /* This strdup macro is NULL safe- copying NULL will yield NULL */

[pacman-dev] [PATCH] Fix clang 8 string-plus-int warnings

[Rikard Falkeborn]

Clang 8 warns that adding a string to an integer does not append to
string. Indeed it doesn't, but that was not the intentetion. Use array
indexing as suggested by the compiler to silence the warning. There
should be no functional change.

Example of warning message:

alpm.c:71:54: warning: adding 'int' to a string does not append to the 
string [-Wstring-plus-int]
sprintf(hookdir, "%s%s", myhandle->root, SYSHOOKDIR + 1);
 ~~~^~~
alpm.c:71:54: note: use array indexing to silence this warning
sprintf(hookdir, "%s%s", myhandle->root, SYSHOOKDIR + 1);
^
 &  [  ]
1 warning generated.
---
 lib/libalpm/alpm.c | 2 +-
 src/pacman/conf.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index 0f3db6f4..bd7a129a 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -68,7 +68,7 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, 
const char *dbpath,
 * with a slash) correctly, we skip SYSHOOKDIR[0]; the regular +1 
therefore
 * disappears from the allocation */
MALLOC(hookdir, strlen(myhandle->root) + strlen(SYSHOOKDIR), goto 
nomem);
-   sprintf(hookdir, "%s%s", myhandle->root, SYSHOOKDIR + 1);
+   sprintf(hookdir, "%s%s", myhandle->root, [1]);
myhandle->hookdirs = alpm_list_add(NULL, hookdir);
 
/* set default database extension */
diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 3b79fbc7..2d8518c4 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -975,11 +975,11 @@ int setdefaults(config_t *c)
if(c->rootdir) {
char path[PATH_MAX];
if(!c->dbpath) {
-   snprintf(path, PATH_MAX, "%s/%s", c->rootdir, DBPATH + 
1);
+   snprintf(path, PATH_MAX, "%s/%s", c->rootdir, 
[1]);
SETDEFAULT(c->dbpath, strdup(path));
}
if(!c->logfile) {
-   snprintf(path, PATH_MAX, "%s/%s", c->rootdir, LOGFILE + 
1);
+   snprintf(path, PATH_MAX, "%s/%s", c->rootdir, 
[1]);
SETDEFAULT(c->logfile, strdup(path));
}
} else {
-- 
2.21.0

[pacman-dev] [PATCH] doc: Remove double spaces

[Rikard Falkeborn]

Signed-off-by: Rikard Falkeborn 
---
 doc/pacman.conf.5.asciidoc  | 2 +-
 doc/submitting-patches.asciidoc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/pacman.conf.5.asciidoc b/doc/pacman.conf.5.asciidoc
index 5f9def5b..9810fc7f 100644
--- a/doc/pacman.conf.5.asciidoc
+++ b/doc/pacman.conf.5.asciidoc
@@ -176,7 +176,7 @@ Options
operation on a local file. Uses the value from SigLevel as the default.
 
 *RemoteFileSigLevel =* ...::
-   Set the signature verification level for installing  packages using the 
"-U"
+   Set the signature verification level for installing packages using the 
"-U"
operation on a remote file URL. Uses the value from SigLevel as the 
default.
 
 *UseSyslog*::
diff --git a/doc/submitting-patches.asciidoc b/doc/submitting-patches.asciidoc
index 73c7487d..9cf8dc64 100644
--- a/doc/submitting-patches.asciidoc
+++ b/doc/submitting-patches.asciidoc
@@ -39,7 +39,7 @@ address if you're afraid of spam.
 
 * Describe your patch.
 
-It helps if you describe the overview and goals  of the patch in the git commit
+It helps if you describe the overview and goals of the patch in the git commit
 log.  This allows others to see what you intended so as to compare it to what
 was actually done, and allows better feedback.
 
-- 
2.18.0

[pacman-dev] [PATCH 1/3] PKGBUILD.5: Mention valid characters for arch

[Rikard Falkeborn]

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 doc/PKGBUILD.5.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt
index 18bc2a19..3b8c2ed1 100644
--- a/doc/PKGBUILD.5.txt
+++ b/doc/PKGBUILD.5.txt
@@ -168,7 +168,8 @@ contain whitespace characters.
 *arch (array)*::
Defines on which architectures the given package is available (e.g.,
`arch=('i686' 'x86_64')`). Packages that contain no architecture 
specific
-   files should use `arch=('any')`.
+   files should use `arch=('any')`. Valid characters for members of this 
array
+   are alphanumerics and ```_`''.
 
 *backup (array)*::
An array of file names, without preceding slashes, that
-- 
2.16.2

[pacman-dev] [PATCH 2/3] PKGBUILD.5: Mention that pkgver can't contain whitespace

[Rikard Falkeborn]

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 doc/PKGBUILD.5.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt
index 3b8c2ed1..cbd02e88 100644
--- a/doc/PKGBUILD.5.txt
+++ b/doc/PKGBUILD.5.txt
@@ -48,7 +48,8 @@ similar to `$_basekernver`.
 
 *pkgver*::
The version of the software as released from the author (e.g., '2.7.1').
-   The variable is not allowed to contain colons, forward slashes or 
hyphens.
+   The variable is not allowed to contain colons, forward slashes, hyphens
+   or whitespace.
 +
 The `pkgver` variable can be automatically updated by providing a `pkgver()`
 function in the PKGBUILD that outputs the new package version.
-- 
2.16.2

[pacman-dev] [PATCH 0/3] Make documentation and PKGBUILD linting more consistent

[Rikard Falkeborn]

Make the documentation a little more explicit about allowed characters
for arch and pkgver. Also fix linting of pkgver which was broken if
pkgver contained whitespace.

Rikard Falkeborn (3):
  PKGBUILD.5: Mention valid characters for arch
  PKGBUILD.5: Mention that pkgver can't contain whitespace
  Fix linting of whitespace in pkgver

 doc/PKGBUILD.5.txt| 6 --
 scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.16.2

[pacman-dev] [PATCH 3/3] Fix linting of whitespace in pkgver

[Rikard Falkeborn]

In order to detect if pkgver contains whitespace, we need to quote it.
Previously, only the characters up to the first whitespace was checked.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in 
b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
index 0ec6584c..2325b1f5 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
@@ -47,5 +47,5 @@ lint_pkgver() {
return 0
fi
 
-   check_pkgver $pkgver
+   check_pkgver "$pkgver"
 }
-- 
2.16.2

[pacman-dev] [PATCH] Suppress valgrind error in fakechroot

[Rikard Falkeborn]

This was spotted on Arch Linux, most likely it was introduced
when fakechroot was updated to 2.19-1.

Valgrind suggests to add the following suppression, which can be
tweaked to fit an already existing one.

  {
 
 Memcheck:Leak
 match-leak-kinds: reachable
 fun:calloc
 obj:/usr/lib/libdl-2.24.so
 fun:dlsym
 obj:/usr/lib/libfakeroot/fakechroot/libfakechroot.so
 fun:bindtextdomain
 obj:/usr/lib/libgpg-error.so.0.20.0
 fun:call_init.part.0
 fun:_dl_init
 obj:/usr/lib/ld-2.24.so
 obj:*
 obj:*
 obj:*
  }

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 valgrind.supp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/valgrind.supp b/valgrind.supp
index 0691e808..69265dff 100644
--- a/valgrind.supp
+++ b/valgrind.supp
@@ -160,8 +160,10 @@
fakechroot-call-init
Memcheck:Leak
match-leak-kinds: reachable
-   fun:malloc
+   fun:?alloc
+   ...
obj:/usr/lib/libfakeroot/fakechroot/libfakechroot.so
+   ...
fun:call_init.part.0
fun:_dl_init
obj:/usr/lib/ld-*.so
-- 
2.11.0

Re: [pacman-dev] [PATCH v3 1/2] makepkg: Move parseopts from library to libmakepkg

[Rikard Falkeborn]

>> Why not move it to util/parseopts.sh if it should be moved into
libmakepkg?
>>
> I suppose that makes sense. util/util.sh ("General utility functions")
may also be appropriate if we don't want to create a new file.

Why do you want to move it to libmakepkg? Isn't library a more suitable
place? From the README: "This directory contains code snippets that can be
reused by multiple scripts"

Rikard

Re: [pacman-dev] [PATCH 1/1] lint_pkgbuild/pkgname: pkgname is not allowed to be empty

[Rikard Falkeborn]

2016-10-04 10:21 GMT+02:00 Christian Hesse :

> From: Christian Hesse 
>
> We checked for empty array elements, but did not catch empty array. Add
> a check for that case as well.
>
> Signed-off-by: Christian Hesse 
> ---
>  scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
> b/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
> index a044082..2696afa 100644
> --- a/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
> +++ b/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
> @@ -32,6 +32,11 @@ lint_pkgbuild_functions+=('lint_pkgname')
>  lint_pkgname() {
> local ret=0 i
>
> +   if [[ -z "${pkgname[0]}" ]]; then
> +   error "$(gettext "%s is not allowed to be empty.")"
> "pkgname"
> +   ret=1
> +   fi
> +
> for i in "${pkgname[@]}"; do
> if [[ -z $i ]]; then
> error "$(gettext "%s is not allowed to be
> empty.")" "pkgname"
> --
> 2.10.0
>

If pkgname="", this will cause the error message to be printed twice (once
in the added if-statement and once in the loop). Should we just return
immediately instead of setting ret=1?

/Rikard

Re: [pacman-dev] [PATCH v2] makepkg: Move parseopts from library to libmakepkg

[Rikard Falkeborn]

2016-10-08 7:18 GMT+02:00 Alad Wenter :

> Signed-off-by: Alad Wenter 
> ---
>  v2: Add missing signoff.
>
>  scripts/libmakepkg/util/option.sh | 157 ++
> 
>  scripts/library/README|  20 -
>  scripts/library/parseopts.sh  | 137 -
>  scripts/makepkg.sh.in |   2 -
>  4 files changed, 157 insertions(+), 159 deletions(-)
>  delete mode 100644 scripts/library/parseopts.sh
>

parseopts.sh is used in a couple of other scripts, is referenced in a
couple of makefiles, and even has test cases. Each of these would need to
be changed. See below.

Also, IMO, for a change like this, a short motivation should go into the
commit message (one sentence may be enough).

Moving parseopts to libmakepkg would make a couple of "pacman"-scripts
depend on libmakepkg (instead of library), which feels wrong, but feel free
to ignore me, I'm not a pacman dev. :)

 [~/code/pacman]./autogen.sh && ./configure && make
 ...
 make[3]: *** No rule to make target 'library/parseopts.sh', needed by
'makepkg'.  Stop.
 make[2]: *** [Makefile:623: all-recursive] Error 1
 make[1]: *** [Makefile:969: all-recursive] Error 1

 git grep parseopts\.sh -- '*' ':!scripts/po/*'
 contrib/Makefile.am:paccache: $(srcdir)/paccache.sh.in
$(top_srcdir)/scripts/library/parseopts.sh
$(top_srcdir)/scripts/library/size_to_human.sh
 contrib/bacman.sh.in:m4_include(../scripts/library/parseopts.sh)
 contrib/paccache.sh.in:m4_include(../scripts/library/parseopts.sh)
 scripts/Makefile.am:library/parseopts.sh \
 scripts/Makefile.am:$(srcdir)/library/parseopts.sh \
 scripts/Makefile.am:$(srcdir)/library/parseopts.sh
 scripts/Makefile.am:$(srcdir)/library/parseopts.sh
 scripts/Makefile.am:$(srcdir)/library/parseopts.sh \
 scripts/libmakepkg/util/option.sh:# parseopts.sh:
 scripts/pacman-db-upgrade.sh.in:m4_include(library/parseopts.sh)
 scripts/pacman-key.sh.in:m4_include(library/parseopts.sh)
 scripts/pkgdelta.sh.in:m4_include(library/parseopts.sh)
 test/scripts/parseopts_test.sh:lib=${1:-${PMTEST_SCRIPTLIB_DIR}parseopts.sh}

/Rikard

[pacman-dev] [PATCH 2/2] Change type of count in be_sync

[Rikard Falkeborn]

Making it size_t matches the return value of alpm_list_count() and
avoids the implicit cast to int.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 lib/libalpm/be_sync.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index f127ed8..079db19 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -452,8 +452,8 @@ static size_t estimate_package_count(struct stat *st, 
struct archive *archive)
 static int sync_db_populate(alpm_db_t *db)
 {
const char *dbpath;
-   size_t est_count;
-   int count, fd;
+   size_t est_count, count;
+   int fd;
int ret = 0;
struct stat buf;
struct archive *archive;
@@ -510,10 +510,10 @@ static int sync_db_populate(alpm_db_t *db)
count = alpm_list_count(db->pkgcache->list);
if(count > 0) {
db->pkgcache->list = alpm_list_msort(db->pkgcache->list,
-   (size_t)count, _alpm_pkg_cmp);
+   count, _alpm_pkg_cmp);
}
_alpm_log(db->handle, ALPM_LOG_DEBUG,
-   "added %d packages to package cache for db '%s'\n",
+   "added %zu packages to package cache for db '%s'\n",
count, db->treename);
 
 cleanup:
-- 
2.10.0

[pacman-dev] [PATCH 1/2] Return boolean from db_populate

[Rikard Falkeborn]

Since the number of packages is not used anywhere, just return a
boolean to avoid the implicit cast from size_t to int in be_local.c.
Use 0 as success to be consistent with db_validate.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---

This patch is made on top of the "Fix Gcc overflow patch". It will
apply to master directly, but the diff doesn't match fully.

As suggested by Andrew after concern of casts from size_t to
int, triggered by the "Fix Gcc overflow patch", change the
"populate" functions to return a flag instead of the number
of packages to avoid implicit casting from size_t to int
in local_db_populate().

The next patch changes the sync_db_populate() function to use
size_t for count in order to not implicitly cast the return
value from alpm_list_count() to int.

 lib/libalpm/be_local.c | 2 +-
 lib/libalpm/be_sync.c  | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 3ef1975..43c6bc9 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -612,7 +612,7 @@ static int local_db_populate(alpm_db_t *db)
_alpm_log(db->handle, ALPM_LOG_DEBUG, "added %zu packages to package 
cache for db '%s'\n",
count, db->treename);
 
-   return count;
+   return 0;
 }
 
 /* Note: the return value must be freed by the caller */
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 32a669d..f127ed8 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -454,6 +454,7 @@ static int sync_db_populate(alpm_db_t *db)
const char *dbpath;
size_t est_count;
int count, fd;
+   int ret = 0;
struct stat buf;
struct archive *archive;
struct archive_entry *entry;
@@ -487,7 +488,7 @@ static int sync_db_populate(alpm_db_t *db)
db->pkgcache = _alpm_pkghash_create(est_count);
if(db->pkgcache == NULL) {
db->handle->pm_errno = ALPM_ERR_MEMORY;
-   count = -1;
+   ret = -1;
goto cleanup;
}
 
@@ -520,7 +521,7 @@ cleanup:
if(fd >= 0) {
close(fd);
}
-   return count;
+   return ret;
 }
 
 /* This function validates %FILENAME%. filename must be between 3 and
-- 
2.10.0

[pacman-dev] [PATCH] Fix gcc strict-overflow error

[Rikard Falkeborn]

Recent gcc (tested with 6.2.1) produces the following error when
compiling with both --enable-warningflags and --enable-debug.
In particular, it seems it is the combination of GCC_STACK_PROTECT_LIB
and -Wstrict-overflow=5 produces the error.

be_local.c:609:4: error: assuming signed overflow does not occur
 when simplifying conditional
 [-Werror=strict-overflow]
if(count > 0) {

Fix this by changing the type of count from int to size_t, which is
fine since count is never negative.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 lib/libalpm/be_local.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index cc2d8ba..3ef1975 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -501,7 +501,7 @@ version_error:
 static int local_db_populate(alpm_db_t *db)
 {
size_t est_count;
-   int count = 0;
+   size_t count = 0;
struct stat buf;
struct dirent *ent = NULL;
const char *dbpath;
@@ -607,9 +607,9 @@ static int local_db_populate(alpm_db_t *db)
 
closedir(dbdir);
if(count > 0) {
-   db->pkgcache->list = alpm_list_msort(db->pkgcache->list, 
(size_t)count, _alpm_pkg_cmp);
+   db->pkgcache->list = alpm_list_msort(db->pkgcache->list, count, 
_alpm_pkg_cmp);
}
-   _alpm_log(db->handle, ALPM_LOG_DEBUG, "added %d packages to package 
cache for db '%s'\n",
+   _alpm_log(db->handle, ALPM_LOG_DEBUG, "added %zu packages to package 
cache for db '%s'\n",
count, db->treename);
 
return count;
-- 
2.9.3

Re: [pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

[Rikard Falkeborn]

2016-06-09 23:16 GMT+02:00 Tobias Stoeckmann :

> ...
> You can trigger it by running this command:
>
> $ echo -e "-.pkg.tar.xz\n-.pkg.tar.xz" | pacsort -k 99 -t '\0'
> Segmentation fault
>
> ...


> $ echo "-/.pkg.tar.xz" | pacsort
> Segmentation fault
>
>
Could these two cases be added to pacman/test/util/pacsorttest.sh?

Rikard

Re: [pacman-dev] [PATCH] alpm: Fix possible alignment issues w/ events

[Rikard Falkeborn]

2016-01-02 19:21 GMT+01:00 Olivier Brunel <j...@jjacky.com>:

> As reported by Rikard Falkeborn[1] using event-specific struct and then
> typecasting to the generic alpm_event_t could possibly lead to alignment
> issue,
> so we now always use alpm_event_t instead.
>
> [1]
> https://lists.archlinux.org/pipermail/pacman-dev/2015-December/020709.html
>
> Signed-off-by: Olivier Brunel <j...@jjacky.com>
> ---
>  lib/libalpm/add.c | 36 ++--
>  lib/libalpm/be_sync.c |  4 ++--
>  lib/libalpm/handle.h  |  2 +-
>  lib/libalpm/hook.c| 14 +++---
>  lib/libalpm/remove.c  | 20 ++--
>  lib/libalpm/sync.c|  8 
>  lib/libalpm/trans.c   |  2 +-
>  lib/libalpm/util.c|  4 ++--
>  8 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 63eda49..4b76bc7 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -315,12 +315,12 @@ static int extract_single_file(alpm_handle_t
> *handle, struct archive *archive,
> }
>
> if(notouch) {
> -   alpm_event_pacnew_created_t event = {
> +   alpm_event_t event = {
> .type = ALPM_EVENT_PACNEW_CREATED,
> -   .from_noupgrade = 1,
> -   .oldpkg = oldpkg,
> -   .newpkg = newpkg,
> -   .file = filename
> +   .pacnew_created.from_noupgrade = 1,
> +   .pacnew_created.oldpkg = oldpkg,
> +   .pacnew_created.newpkg = newpkg,
> +   .pacnew_created.file = filename
> };
> /* "remove" the .pacnew suffix */
> filename[filename_len] = '\0';
> @@ -366,12 +366,12 @@ static int extract_single_file(alpm_handle_t
> *handle, struct archive *archive,
> } else {
> /* none of the three files matched another,  leave
> the unpacked
>  * file alongside the local file */
> -   alpm_event_pacnew_created_t event = {
> +   alpm_event_t event = {
> .type = ALPM_EVENT_PACNEW_CREATED,
> -   .from_noupgrade = 0,
> -   .oldpkg = oldpkg,
> -   .newpkg = newpkg,
> -   .file = origfile
> +   .pacnew_created.from_noupgrade = 0,
> +   .pacnew_created.oldpkg = oldpkg,
> +   .pacnew_created.newpkg = newpkg,
> +   .pacnew_created.file = origfile
> };
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "action: keeping current file and
> installing"
> @@ -398,7 +398,7 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> alpm_db_t *db = handle->db_local;
> alpm_trans_t *trans = handle->trans;
> alpm_progress_t progress = ALPM_PROGRESS_ADD_START;
> -   alpm_event_package_operation_t event;
> +   alpm_event_t event;
> const char *log_msg = "adding";
> const char *pkgfile;
>
> @@ -411,15 +411,15 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> if(cmp < 0) {
> log_msg = "downgrading";
> progress = ALPM_PROGRESS_DOWNGRADE_START;
> -   event.operation = ALPM_PACKAGE_DOWNGRADE;
> +   event.package_operation.operation =
> ALPM_PACKAGE_DOWNGRADE;
> } else if(cmp == 0) {
> log_msg = "reinstalling";
> progress = ALPM_PROGRESS_REINSTALL_START;
> -   event.operation = ALPM_PACKAGE_REINSTALL;
> +   event.package_operation.operation =
> ALPM_PACKAGE_REINSTALL;
> } else {
> log_msg = "upgrading";
> progress = ALPM_PROGRESS_UPGRADE_START;
> -   event.operation = ALPM_PACKAGE_UPGRADE;
> +   event.package_operation.operation =
> ALPM_PACKAGE_UPGRADE;
> }
> is_upgrade = 1;
>
> @@ -432,12 +432,12 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> /* copy over the install reason */
> newpkg->reason = alpm_pkg_get_reason(local);
> 

Re: [pacman-dev] Clang warnings

[Rikard Falkeborn]

Den 1 jan 2016 15:06 skrev "Olivier Brunel" <j...@jjacky.com>:
>
> On Fri, 1 Jan 2016 23:03:22 +1000
> Allan McRae <al...@archlinux.org> wrote:
>
> > On 01/01/16 22:01, Olivier Brunel wrote:
> > > On Thu, 31 Dec 2015 14:32:45 +0100
> > > Rikard Falkeborn <rikard.falkeb...@gmail.com> wrote:
> > >
> > >> There are some warnings recently introduced when compiling with
> > >> Clang (with --enable-warningflags) for x86_64. The warnings are
> > >> related to casting of specific alpm_event-types to alpm_event_t
> > >> where the required alignment differs between the types. On x86, it
> > >> shouldn't be a problem, but maybe there are other architectures
> > >> pacman should work on where this is a real issue?
> > >> Either way, I think it would be good if pacman built cleanly with
> > >> Clang. Any suggestions on how to fix this?
> > >
> > > I think the only way is not to use a specific event type but the
> > > generic one, to ensure the size is correct. I don't have clang so I
> > > didn't test this, but I think there are two ways to solve this.
> > >
> > > Basically the issue is when using code such as:
> > >
> > > alpm_event_hook_run_t event;
> > > event.type = ALPM_EVENT_HOOK_RUN_START;
> > > event.name = name;
> > > event.desc = desc;
> > > EVENT(handle, );
> > >
> > > So we could either use the alpm_event_t and always go into the right
> > > union member, e.g:
> > >
> > > alpm_event_t event;
> > > event.hook_run.type = ALPM_EVENT_HOOK_RUN_START;
> > > event.hook_run.name = name;
> > > event.hook_run.desc = desc;
> > > EVENT(handle, );
> > >
> >
> > I'd prefer this way but...
> >
> > What is different with these events than all the others? Why do the
> > old ones not give warnings? We should be consistent here. I have a
> > feeling we have had this conversation before...
> >
> > Allan
>
> I picked hook_run as an example, but it might affect older ones as
> well. The point is that alpm_event_t and alpm_event_hook_run_t differ
> in size. Now since the former is a union of all the specific events, it
> can hold them all, but when creating/using only a specific one, it
> might be of a different size.
> (Or maybe this is the first one to grow over the generic event?)
>
> As for having had this conversation before, I'm not sure, but there
> probaly have been a similar one in the other way around: how to use the
> generic event to get specific values, e.g. in pacman callback handler:
> typecast, of through the union.
>
> For the record, though sometimes we just go through the union (e.g.
> event->scriptlet_info.line) most times (or, whenever more than one or
> two values are needed, or used more than once) we just use a
> specific pointer, e.g: alpm_event_hook_run_t *e = >hook_run;
>
> -j

The types clang warns about do not contain pointers, I think every other
type does, that's why the required alignment differs. Adding a dummy
pointer to them makes the warnings go away.

The warning has nothing to do with the actual size of the types but where
in memory they are stored (is the adress guaranteed to be an even multiple
of four or eight).

Rikard

[pacman-dev] Clang warnings

[Rikard Falkeborn]

There are some warnings recently introduced when compiling with Clang (with
--enable-warningflags) for x86_64. The warnings are related to casting of
specific alpm_event-types to alpm_event_t where the required alignment
differs between the types. On x86, it shouldn't be a problem, but maybe
there are other architectures pacman should work on where this is a real
issue?
Either way, I think it would be good if pacman built cleanly with Clang. Any
suggestions on how to fix this?

For reference, the warnings are:

hook.c:732:3: warning: cast from 'alpm_event_hook_t *' (aka 'struct
_alpm_event_hook_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *')
increases required alignment from 4 to 8 [-Wcast-align]
EVENT(handle, );
^
./handle.h:37:16: note: expanded from macro 'EVENT'
(h)->eventcb((alpm_event_t *) (e)); \
 ^~~~
hook.c:761:3: warning: cast from 'alpm_event_hook_t *' (aka 'struct
_alpm_event_hook_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *')
increases required alignment from 4 to 8 [-Wcast-align]
EVENT(handle, );
^
./handle.h:37:16: note: expanded from macro 'EVENT'
(h)->eventcb((alpm_event_t *) (e)); \
 ^~~~
2 warnings generated.
trans.c:202:2: warning: cast from 'alpm_event_any_t *' (aka 'struct
_alpm_event_any_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *')
increases required alignment from 4 to 8 [-Wcast-align]
EVENT(handle, );
^
./handle.h:37:16: note: expanded from macro 'EVENT'
(h)->eventcb((alpm_event_t *) (e)); \
 ^~~~
trans.c:226:3: warning: cast from 'alpm_event_any_t *' (aka 'struct
_alpm_event_any_t *') to 'alpm_event_t *' (aka 'union _alpm_event_t *')
increases required alignment from 4 to 8 [-Wcast-align]
EVENT(handle, );
^
./handle.h:37:16: note: expanded from macro 'EVENT'
(h)->eventcb((alpm_event_t *) (e)); \
 ^~~~

[pacman-dev] [PATCH 3/3] Add pacsort tests with invalid input

[Rikard Falkeborn]

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 test/util/pacsorttest.sh | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/test/util/pacsorttest.sh b/test/util/pacsorttest.sh
index 3b285b8..32ee6ba 100755
--- a/test/util/pacsorttest.sh
+++ b/test/util/pacsorttest.sh
@@ -35,7 +35,16 @@ tap_runtest() {
tap_diff <(printf "$1" | $bin $4) <(printf "$2") "$3"
 }
 
-tap_plan 26
+# args:
+# check_return_value input expected_return_value test_description optional_opts
+tap_check_return_value() {
+# run the test
+printf "$1" | $bin $4 2>/dev/null
+tap_is_int "$?" "$2" "$3"
+
+}
+
+tap_plan 32
 
 in="1\n2\n3\n4\n"
 tap_runtest $in $in "already ordered"
@@ -108,6 +117,13 @@ tap_runtest "$separator_reverse" "$separator" "really long 
input, sort key, sepa
 tap_runtest "$separator_reverse" "$separator_reverse" "really long input, sort 
key, separator, reversed" "-k 3 -t| -r"
 tap_runtest "$separator" "$separator_reverse" "really long input, sort key, 
separator, reversed" "-k 3 -t| -r"
 
+tap_check_return_value "" "2" "invalid sort key (no argument)" "-k"
+tap_check_return_value "" "2" "invalid sort key (non-numeric)" "-k asd"
+tap_check_return_value "" "2" "invalid field separator (no argument)" "-t"
+tap_check_return_value "" "2" "invalid field separator (multiple characters)" 
"-t sda"
+tap_check_return_value "" "2" "invalid field separator (two characters must 
start with a slash)" "-t ag"
+tap_check_return_value "" "2" "invalid field separator (\g is invalid)" '-t \g'
+
 tap_finish
 
 # vim: set noet:
-- 
2.6.4

[pacman-dev] [PATCH 2/3] pacsort, introduce define for escape_char error code

[Rikard Falkeborn]

The signedness of char is implementation defined. On systems where
char is unsigned, comparing a variable of type char with -1 is never
true, due to integer promotion rules. To avoid this, introduce a
define for invalid field separators where -1 is cast to char. This will
ensure that the return value check works for both unsigned and signed char.

Fixes one warning [-Wtype-limits] for comparissons with -1 when compiling
with -funsigned-char.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 src/util/pacsort.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/util/pacsort.c b/src/util/pacsort.c
index e7dc63e..dcf1ade 100644
--- a/src/util/pacsort.c
+++ b/src/util/pacsort.c
@@ -28,6 +28,7 @@
 #include "util-common.h"
 
 #define DELIM ' '
+#define INVALD_ESCAPE_CHAR ((char)-1)
 
 #ifndef MIN
 #define MIN(a, b)  \
@@ -385,13 +386,13 @@ static int vercmp(const void *p1, const void *p2)
 static char escape_char(const char *string)
 {
if(!string) {
-   return -1;
+   return INVALD_ESCAPE_CHAR;
}
 
const size_t len = strlen(string);
 
if(len > 2) {
-   return -1;
+   return INVALD_ESCAPE_CHAR;
}
 
if(len == 1) {
@@ -399,7 +400,7 @@ static char escape_char(const char *string)
}
 
if(*string != '\\') {
-   return -1;
+   return INVALD_ESCAPE_CHAR;
}
 
switch(string[1]) {
@@ -412,7 +413,7 @@ static char escape_char(const char *string)
case '0':
return '\0';
default:
-   return -1;
+   return INVALD_ESCAPE_CHAR;
}
 }
 
@@ -463,7 +464,7 @@ static int parse_options(int argc, char **argv)
break;
case 't':
opts.delim = escape_char(optarg);
-   if(opts.delim == -1) {
+   if(opts.delim == INVALD_ESCAPE_CHAR) {
fprintf(stderr, "error: invalid field 
separator -- `%s'\n", optarg);
return 1;
}
-- 
2.6.4

[pacman-dev] [PATCH 0/3] Char signedness issues

[Rikard Falkeborn]

I tried compiling pacman on my RPi (Arm), where char is unsigned by default.
That leads to some things not working the way they are expected too. The
problem is as follows:

char a = -1;
if (a == -1) { ... }

If char is unsigned, we will not enter the if-statement (a is converted to int)
before making the comparisson.

Gcc will warn about these comparissons. The first two patches fixes the
cases I (and Gcc) could find, and the third one is a couple of tests I wrote
to convince myself there was a problem in pacsort (and more importantly, that
the changes actually fixes something).

After doing this, I realize that the simplest solution probably would be to
add -fsigned-char as an argument to the compiler... thoughts?

Rikard Falkeborn (3):
  Make alpm_graph state signedness explicit
  pacsort, introduce define for escape_char error code
  Add pacsort tests with invalid input

 lib/libalpm/graph.h  |  2 +-
 src/util/pacsort.c   | 11 ++-
 test/util/pacsorttest.sh | 18 +-
 3 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.6.4

[pacman-dev] [PATCH 1/3] Make alpm_graph state signedness explicit

[Rikard Falkeborn]

The signedness of char is implementation defined. Since the
alpm_graph state is clearly meant to be signed, make the
signedness explicit.

This fixes bugs on systems where char is unsigned, in comparissons
of the following type:

  if(v.state == -1)

which, if state is unsigned, will never be true due to integer
promotion rules.

Fixes failing test/pacman/tests/sync012.py when compiling with -funsigned-char.

Fixes two warnings [-Wtype-limits] for comparissons with -1 when compiling
with -funsigned-char.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 lib/libalpm/graph.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libalpm/graph.h b/lib/libalpm/graph.h
index fb24d73..ae0b5d9 100644
--- a/lib/libalpm/graph.h
+++ b/lib/libalpm/graph.h
@@ -29,7 +29,7 @@ typedef struct __alpm_graph_t {
alpm_list_t *children;
alpm_list_t *childptr; /* points to a child in children list */
off_t weight; /* weight of the node */
-   char state; /* 0: untouched, -1: entered, other: leaving time */
+   signed char state; /* 0: untouched, -1: entered, other: leaving time */
 } alpm_graph_t;
 
 alpm_graph_t *_alpm_graph_new(void);
-- 
2.6.4

[pacman-dev] [PATCH] libalpm, fix leak if malloc fails in be_sync

[Rikard Falkeborn]

---
 lib/libalpm/be_sync.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index b09b060..9229622 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -178,6 +178,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
const char *dbext;
alpm_list_t *i;
int ret = -1;
+   int memory_error = 0;
mode_t oldmask;
alpm_handle_t *handle;
alpm_siglevel_t level;
@@ -230,8 +231,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
/* print server + filename into a buffer */
len = strlen(server) + strlen(db->treename) + strlen(dbext) + 2;
-   /* TODO fix leak syncpath and umask unset */
-   MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, 
-1));
+   MALLOC(payload.fileurl, len, memory_error = 1; goto cleanup);
snprintf(payload.fileurl, len, "%s/%s%s", server, db->treename, 
dbext);
payload.handle = handle;
payload.force = force;
@@ -269,8 +269,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
len = strlen(server) + strlen(db->treename) + 
strlen(dbext) + 6;
}
 
-   /* TODO fix leak syncpath and umask unset */
-   MALLOC(payload.fileurl, len, RET_ERR(handle, 
ALPM_ERR_MEMORY, -1));
+   MALLOC(payload.fileurl, len, memory_error = 1; goto 
cleanup);
 
if(final_db_url != NULL) {
snprintf(payload.fileurl, len, "%s.sig", 
final_db_url);
@@ -322,9 +321,10 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
}
 
 cleanup:
-   _alpm_handle_unlock(handle);
free(syncpath);
umask(oldmask);
+   ASSERT(memory_error == 0, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+   _alpm_handle_unlock(handle);
return ret;
 }
 
-- 
2.6.2

[pacman-dev] [PATCH] Use correct format specifiers

[Rikard Falkeborn]

---
 lib/libalpm/diskspace.c |  2 +-
 lib/libalpm/dload.c |  2 +-
 lib/libalpm/remove.c|  2 +-
 lib/libalpm/util.c  |  2 +-
 src/pacman/callback.c   | 12 ++--
 src/pacman/conf.c   |  4 ++--
 src/pacman/util.c   |  4 ++--
 7 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c
index aaa0036..5bb2173 100644
--- a/lib/libalpm/diskspace.c
+++ b/lib/libalpm/diskspace.c
@@ -349,7 +349,7 @@ static int check_mountpoint(alpm_handle_t *handle, 
alpm_mountpoint_t *mp)
(uintmax_t)cushion, (uintmax_t)mp->fsp.f_bfree);
if(needed >= 0 && (fsblkcnt_t)needed > mp->fsp.f_bfree) {
_alpm_log(handle, ALPM_LOG_ERROR,
-   _("Partition %s too full: %jd blocks needed, 
%jd blocks free\n"),
+   _("Partition %s too full: %jd blocks needed, 
%ju blocks free\n"),
mp->mount_dir, (intmax_t)needed, 
(uintmax_t)mp->fsp.f_bfree);
return 1;
}
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 63523aa..f3df9cd 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -323,7 +323,7 @@ static void curl_set_handle_opts(struct dload_payload 
*payload,
curl_easy_setopt(curl, CURLOPT_TIMECONDITION, 
CURL_TIMECOND_IFMODSINCE);
curl_easy_setopt(curl, CURLOPT_TIMEVALUE, (long)st.st_mtime);
_alpm_log(handle, ALPM_LOG_DEBUG,
-   "using time condition: %lu\n", 
(long)st.st_mtime);
+   "using time condition: %ld\n", 
(long)st.st_mtime);
} else if(stat(payload->tempfile_name, ) == 0 && 
payload->allow_resume) {
/* a previous partial download exists, resume from end of file. 
*/
payload->tempfile_openmode = "ab";
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index bc5713a..2fb7993 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -599,7 +599,7 @@ static int remove_package_files(alpm_handle_t *handle,
}
}
 
-   _alpm_log(handle, ALPM_LOG_DEBUG, "removing %zd files\n", 
filelist->count);
+   _alpm_log(handle, ALPM_LOG_DEBUG, "removing %zu files\n", 
filelist->count);
 
if(!newpkg) {
/* init progress bar, but only on true remove transactions */
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 66a2742..955bf6f 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -1290,7 +1290,7 @@ void *_alpm_greedy_grow(void **data, size_t *current, 
const size_t required)
 
 void _alpm_alloc_fail(size_t size)
 {
-   fprintf(stderr, "alloc failure: could not allocate %zd bytes\n", size);
+   fprintf(stderr, "alloc failure: could not allocate %zu bytes\n", size);
 }
 
 /* vim: set noet: */
diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 9260ece..7a21b22 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -418,8 +418,8 @@ void cb_question(alpm_question_t *question)
alpm_question_select_provider_t *q = 
>select_provider;
size_t count = alpm_list_count(q->providers);
char *depstring = 
alpm_dep_compute_string(q->depend);
-   colon_printf(_n("There is %zd provider 
available for %s\n",
-   "There are %zd providers 
available for %s:\n", count),
+   colon_printf(_n("There is %zu provider 
available for %s\n",
+   "There are %zu providers 
available for %s:\n", count),
count, depstring);
free(depstring);
select_display(q->providers);
@@ -443,10 +443,10 @@ void cb_question(alpm_question_t *question)
strftime(created, 12, "%Y-%m-%d", 
localtime());
 
if(q->key->revoked) {
-   q->import = yesno(_("Import PGP key 
%d%c/%s, \"%s\", created: %s (revoked)?"),
+   q->import = yesno(_("Import PGP key 
%u%c/%s, \"%s\", created: %s (revoked)?"),
q->key->length, 
q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created);
} else {
-   q->import = yesno(_("Import PGP key 
%d%c/%s, \"%s\", created: %s?"),
+   q->import = yesno(_("Import PGP key 
%u%c/%s, \"%s\", created: %s?"),
q->key->length, 
q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created);
}
}
@@ -590,8 +590,8 @@ void 

Re: [pacman-dev] [PATCH] package.c, fix incorrect buffersize

[Rikard Falkeborn]

Den 2 nov 2015 08:47 skrev "Pierre Neidhardt" :
>
> Since you caught the issue, Rikard, do you want to do send the patch?
>
> --
> Pierre Neidhardt

Was there a problem with the original patch I sent? If it needs changes or
be resubmitted, I wont be able to do it within a couple of days so feel
free to do it if you like.

Rikard

Re: [pacman-dev] [PATCH] package.c, fix incorrect buffersize

[Rikard Falkeborn]

Den 1 nov 2015 09:55 skrev "Rikard Falkeborn" <rikard.falkeb...@gmail.com>:
>
>
> Den 1 nov 2015 09:36 skrev "Pierre Neidhardt" <ambre...@gmail.com>:
> >
> > There is a much simpler fix:
> >
> > > - static const wchar_t *title_suffix = L" :";
> > > - static const wchar_t title_suffix[] = L" :";
> >
> > I'll commit the patch right row.
> >
> > --
> > Pierre Neidhardt
> >
> > To communicate is the beginning of understanding.
> > -- AT
>
> That's simpler. :)
>
> Can you check if clang is happy with the fix? The reason I found it in
the first place was a clang warning. I don't have a computer nearby to test
it on myself at the moment.
>
> /Rikard

Simpler but not correct I think. We don't want the size in bytes, we want
the number of elements. So making it an array and using ARRAYSIZE would do
the trick. Provided clang doesn't warn...  :)

Rikard

Re: [pacman-dev] [PATCH] package.c, fix incorrect buffersize

[Rikard Falkeborn]

Den 1 nov 2015 09:36 skrev "Pierre Neidhardt" :
>
> There is a much simpler fix:
>
> > - static const wchar_t *title_suffix = L" :";
> > - static const wchar_t title_suffix[] = L" :";
>
> I'll commit the patch right row.
>
> --
> Pierre Neidhardt
>
> To communicate is the beginning of understanding.
> -- AT

That's simpler. :)

Can you check if clang is happy with the fix? The reason I found it in the
first place was a clang warning. I don't have a computer nearby to test it
on myself at the moment.

/Rikard

[pacman-dev] [PATCH] package.c, fix incorrect buffersize

[Rikard Falkeborn]

Correct title_suffix_len to be the actual number of elements in
the string (including the NUL-terminator) instead of the size
of a pointer.

Note that wmemcpy blindly copies the number of wide characters it is told
to copy (no check for NUL-terminating character), so this previously copied
data outside of title_suffix.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 src/pacman/package.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/pacman/package.c b/src/pacman/package.c
index dbd23f5..59f4327 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -79,6 +79,8 @@ enum {
  * potential growth.
  */
 #define TITLE_MAXLEN 50
+#define TITLE_SUFFIX L" :"
+#define TITLE_SUFFIX_LEN (ARRAYSIZE(TITLE_SUFFIX))
 
 static char titles[_T_MAX][TITLE_MAXLEN * sizeof(wchar_t)];
 
@@ -90,9 +92,7 @@ static void make_aligned_titles(void)
 {
unsigned int i;
size_t max = 0;
-   static const wchar_t *title_suffix = L" :";
-   static const size_t title_suffix_len = sizeof(title_suffix);
-   wchar_t wbuf[ARRAYSIZE(titles)][TITLE_MAXLEN + title_suffix_len];
+   wchar_t wbuf[ARRAYSIZE(titles)][TITLE_MAXLEN + TITLE_SUFFIX_LEN];
size_t wlen[ARRAYSIZE(wbuf)];
char *buf[ARRAYSIZE(wbuf)];
buf[T_ARCHITECTURE] = _("Architecture");
@@ -133,7 +133,7 @@ static void make_aligned_titles(void)
 
for(i = 0; i < ARRAYSIZE(wbuf); i++) {
wmemset(wbuf[i] + wlen[i], L' ', max - wlen[i]);
-   wmemcpy(wbuf[i] + max, title_suffix, title_suffix_len);
+   wmemcpy(wbuf[i] + max, TITLE_SUFFIX, TITLE_SUFFIX_LEN);
wcstombs(titles[i], wbuf[i], sizeof(wbuf[i]));
}
 }
-- 
2.6.2

[pacman-dev] [PATCH] Alpm, check for NULL in free-functions

[Rikard Falkeborn]

Also, use FREE() instead of free() in _alpm_backup_free()
to set the pointers to NULL.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
 lib/libalpm/backup.c   | 7 ---
 lib/libalpm/conflict.c | 2 ++
 lib/libalpm/db.c   | 1 +
 lib/libalpm/delta.c| 1 +
 lib/libalpm/deps.c | 2 ++
 lib/libalpm/graph.c| 1 +
 6 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/backup.c b/lib/libalpm/backup.c
index 5ccec03..3d1d6de 100644
--- a/lib/libalpm/backup.c
+++ b/lib/libalpm/backup.c
@@ -76,9 +76,10 @@ alpm_backup_t *_alpm_needbackup(const char *file, alpm_pkg_t 
*pkg)
 
 void _alpm_backup_free(alpm_backup_t *backup)
 {
-   free(backup->name);
-   free(backup->hash);
-   free(backup);
+   ASSERT(backup != NULL, return);
+   FREE(backup->name);
+   FREE(backup->hash);
+   FREE(backup);
 }
 
 alpm_backup_t *_alpm_backup_dup(const alpm_backup_t *backup)
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 0af3e3a..41b8393 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -68,6 +68,7 @@ error:
  */
 void SYMEXPORT alpm_conflict_free(alpm_conflict_t *conflict)
 {
+   ASSERT(conflict != NULL, return);
FREE(conflict->package2);
FREE(conflict->package1);
FREE(conflict);
@@ -301,6 +302,7 @@ error:
  */
 void SYMEXPORT alpm_fileconflict_free(alpm_fileconflict_t *conflict)
 {
+   ASSERT(conflict != NULL, return);
FREE(conflict->ctarget);
FREE(conflict->file);
FREE(conflict->target);
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index b94d334..bb4c4bd 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -344,6 +344,7 @@ alpm_db_t *_alpm_db_new(const char *treename, int is_local)
 
 void _alpm_db_free(alpm_db_t *db)
 {
+   ASSERT(db != NULL, return);
/* cleanup pkgcache */
_alpm_db_free_pkgcache(db);
/* cleanup server list */
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c
index a43269d..5ea2069 100644
--- a/lib/libalpm/delta.c
+++ b/lib/libalpm/delta.c
@@ -335,6 +335,7 @@ error:
 
 void _alpm_delta_free(alpm_delta_t *delta)
 {
+   ASSERT(delta != NULL, return);
FREE(delta->delta);
FREE(delta->delta_md5);
FREE(delta->from);
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index 34ac8d3..f846b1c 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -37,6 +37,7 @@
 
 void SYMEXPORT alpm_dep_free(alpm_depend_t *dep)
 {
+   ASSERT(dep != NULL, return);
FREE(dep->name);
FREE(dep->version);
FREE(dep->desc);
@@ -63,6 +64,7 @@ error:
 
 void SYMEXPORT alpm_depmissing_free(alpm_depmissing_t *miss)
 {
+   ASSERT(miss != NULL, return);
alpm_dep_free(miss->depend);
FREE(miss->target);
FREE(miss->causingpkg);
diff --git a/lib/libalpm/graph.c b/lib/libalpm/graph.c
index 4d58169..7c3cce1 100644
--- a/lib/libalpm/graph.c
+++ b/lib/libalpm/graph.c
@@ -31,6 +31,7 @@ alpm_graph_t *_alpm_graph_new(void)
 
 void _alpm_graph_free(void *data)
 {
+   ASSERT(data != NULL, return);
alpm_graph_t *graph = data;
alpm_list_free(graph->children);
free(graph);
-- 
2.6.2

[pacman-dev] [PATCH] add_fileconflict, don't free if CALLOC fail

[Rikard Falkeborn]

If CALLOC fails, conflict is NULL, so we shouldn't
attempt to free it's members, since that would
lead to a NULL pointer dereference.

Signed-off-by: Rikard Falkeborn <rikard.falkeb...@gmail.com>
---
Is there a reason none of the alpm_*_free() functions check if the input
is NULL? That would be more robust, and what I think would be expected,
since free() accepts NULL as input.

 lib/libalpm/conflict.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 0af3e3a..823fb67 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -273,7 +273,8 @@ static alpm_list_t *add_fileconflict(alpm_handle_t *handle,
alpm_pkg_t *pkg1, alpm_pkg_t *pkg2)
 {
alpm_fileconflict_t *conflict;
-   CALLOC(conflict, 1, sizeof(alpm_fileconflict_t), goto error);
+   CALLOC(conflict, 1, sizeof(alpm_fileconflict_t),
+  RET_ERR(handle, ALPM_ERR_MEMORY, conflicts));
 
STRDUP(conflict->target, pkg1->name, goto error);
STRDUP(conflict->file, filestr, goto error);
-- 
2.6.2

[pacman-dev] [PATCH 1/3] pacsort: handle failing list_add

[Rikard Falkeborn]

Since it can fail, check the return value.
If it fails, we need to free the memory of the object we wanted
to add to the list.

Signed-off-by: Rikard Falkeborn rikard.falkeb...@gmail.com
---
 src/util/pacsort.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/util/pacsort.c b/src/util/pacsort.c
index b0137ec..003ec07 100644
--- a/src/util/pacsort.c
+++ b/src/util/pacsort.c
@@ -254,7 +254,10 @@ static char *explode(struct buffer_t *buffer, struct 
list_t *list)
while((end = memchr(ptr, linedelim, buffer-mem[buffer-len] - ptr))) {
*end = '\0';
meta = input_new(ptr, end - ptr);
-   list_add(list, meta);
+   if(meta == NULL || list_add(list, meta) != 0) {
+   input_free(meta);
+   return NULL;
+   }
ptr = end + 1;
}
 
@@ -294,6 +297,7 @@ static int splitfile(FILE *stream, struct buffer_t *buffer, 
struct list_t *list)
if(buffer-len) {
struct input_t *meta = input_new(buffer-mem, buffer-len + 1);
if(meta == NULL || list_add(list, meta) != 0) {
+   input_free(meta);
return 1;
}
}
-- 
2.5.0

[pacman-dev] [PATCH 2/3] pacsort: don't overwrite memory if realloc fails

[Rikard Falkeborn]

That makes it impossible to free it later.

Signed-off-by: Rikard Falkeborn rikard.falkeb...@gmail.com
---
 src/util/pacsort.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/util/pacsort.c b/src/util/pacsort.c
index 003ec07..3337d97 100644
--- a/src/util/pacsort.c
+++ b/src/util/pacsort.c
@@ -104,10 +104,11 @@ static void buffer_free(struct buffer_t *buf)
 static int buffer_grow(struct buffer_t *buffer)
 {
size_t newsz = buffer-maxlen * 2.5;
-   buffer-mem = realloc(buffer-mem, newsz * sizeof(char));
-   if(!buffer-mem) {
+   char* new_mem = realloc(buffer-mem, newsz * sizeof(char));
+   if(!new_mem) {
return 1;
}
+   buffer-mem = new_mem;
buffer-maxlen = newsz;
 
return 0;
@@ -136,11 +137,12 @@ static struct list_t *list_new(size_t initial_size)
 static int list_grow(struct list_t *list)
 {
size_t newsz = list-maxcount * 2.5;
-   list-list = realloc(list-list, newsz * sizeof(char *));
-   if(!list-list) {
+   void **new_list = realloc(list-list, newsz * sizeof(char *));
+   if(!new_list) {
return 1;
}
 
+   list-list = new_list;
list-maxcount = newsz;
 
return 0;
-- 
2.5.0

[pacman-dev] [PATCH 3/3] pacsort: clean up if error

[Rikard Falkeborn]

* free memory
* close open file

Signed-off-by: Rikard Falkeborn rikard.falkeb...@gmail.com
---
 src/util/pacsort.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/util/pacsort.c b/src/util/pacsort.c
index 3337d97..e7dc63e 100644
--- a/src/util/pacsort.c
+++ b/src/util/pacsort.c
@@ -484,6 +484,7 @@ int main(int argc, char *argv[])
struct list_t *list;
struct buffer_t *buffer;
size_t i;
+   int ret = 0;
 
/* option defaults */
opts.order = 1;
@@ -507,7 +508,8 @@ int main(int argc, char *argv[])
if(optind == argc) {
if(splitfile(stdin, buffer, list) != 0) {
fprintf(stderr, %s: memory exhausted\n, argv[0]);
-   return ENOMEM;
+   ret = ENOMEM;
+   goto cleanup;
}
} else {
while(optind  argc) {
@@ -515,7 +517,9 @@ int main(int argc, char *argv[])
if(input) {
if(splitfile(input, buffer, list) != 0) {
fprintf(stderr, %s: memory 
exhausted\n, argv[0]);
-   return ENOMEM;
+   fclose(input);
+   ret = ENOMEM;
+   goto cleanup;
}
fclose(input);
} else {
@@ -534,10 +538,11 @@ int main(int argc, char *argv[])
}
}
 
+cleanup:
list_free(list, input_free);
buffer_free(buffer);
 
-   return 0;
+   return ret;
 }
 
 /* vim: set noet: */
-- 
2.5.0