The following have been renamed to avoid shadowing:

* conflict.c
- filestr -> pkgfilestr
- remove -> remove_pkgs
* deps.c
- index -> pkg_index
- remove -> remove_pkgs
- time -> epoch
* diskspace.c
- abort -> should_abort
* handle.c
- handle -> local_handle / new_handle
* package.c
- i -> deps
* sync.c (alpm)
- current -> current_pkg
- remove -> remove_pkgs
- ret -> gpg_ret
- sync -> syncpkg
* trans.c
- handle -> local_handle
* util.c (alpm)
- pipe -> pipefh
- prefix -> entry_prefix
* callback.c
- p -> ptr
* util.c (pacman)
- dup -> p

In some instance duplicate declarations were removed.

Signed-off-by: Sebastian Nowicki <sebnow@gmail.com>
---
This is a follow up for an earlier patch I sent back in December.
Apologies for not following up sooner. The feedback was to split the
patch up. This is the first patch from the split.

The API for alpm_checkdeps is slightly modified (a parameter is
renamed). I'm not sure whether this is a backwards incompatible change
or not.

Sorry for the size, I don't think it can be split up much further
without it turning into a dozen patches.

This patch was generated against the master branch.

lib/libalpm/Makefile.am | 2 +-
lib/libalpm/alpm.h | 2 +-
lib/libalpm/conflict.c | 8 +++---
lib/libalpm/deps.c | 18 ++++++++--------
lib/libalpm/diskspace.c | 8 +++---
lib/libalpm/dload.c | 6 ++--
lib/libalpm/handle.c | 50 +++++++++++++++++++++++-----------------------
lib/libalpm/package.c | 6 ++--
lib/libalpm/sync.c | 37 ++++++++++++++++-----------------
lib/libalpm/trans.c | 18 ++++++++--------
lib/libalpm/util.c | 20 +++++++++---------
src/pacman/Makefile.am | 2 +-
src/pacman/callback.c | 10 ++++----
src/pacman/query.c | 2 +-
src/pacman/sync.c | 4 +--
src/pacman/upgrade.c | 1 -
src/pacman/util.c | 14 ++++++------
17 files changed, 102 insertions(+), 106 deletions(-)

diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
index fb224a5..21f7057 100644
--- a/lib/libalpm/Makefile.am
+++ b/lib/libalpm/Makefile.am
@@ -7,7 +7,7 @@ include_HEADERS = alpm_list.h alpm.h

DEFS = -DLOCALEDIR="@localedir@" @DEFS@

-AM_CFLAGS = -pedantic -D_GNU_SOURCE
+AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE

if ENABLE_VISIBILITY_CC
if DARWIN
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index b08191d..1b08fca 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -457,7 +457,7 @@ typedef enum _pmdepmod_t {
} pmdepmod_t;

alpm_list_t *alpm_checkdeps(alpm_list_t *pkglist, int reversedeps,
- alpm_list_t *remove, alpm_list_t *upgrade);
+ alpm_list_t *remove_pkgs, alpm_list_t *upgrade);
pmpkg_t *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstring);
pmpkg_t *alpm_find_dbs_satisfier(alpm_list_t *dbs, const char *depstring);

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index b4ecd65..f18e3d4 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -382,7 +382,7 @@ static int dir_belongsto_pkg(char *dirpath, pmpkg_t *pkg)
* 1: check every target against every target
* 2: check every target against the filesystem */
alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans,
- alpm_list_t *upgrade, alpm_list_t *remove)
+ alpm_list_t *upgrade, alpm_list_t *remove_pkgs)
{
alpm_list_t *i, *j, *conflicts = NULL;
size_t numtargs = alpm_list_count(upgrade);
@@ -477,7 +477,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans,
int resolved_conflict = 0; /* have we acted on this conflict? */

/* Check remove list (will we remove the conflicting local file?) */
- for(k = remove; k && !resolved_conflict; k = k->next) {
+ for(k = remove_pkgs; k && !resolved_conflict; k = k->next) {
pmpkg_t *rempkg = k->data;
if(rempkg && alpm_list_find_str(alpm_pkg_get_files(rempkg), filestr)) {
_alpm_log(PM_LOG_DEBUG, "local file will be removed, not a conflict: %s
", filestr);
@@ -522,8 +522,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmdb_t *db, pmtrans_t *trans,
FREE(rpath);
continue;
}
- char *filestr = rpath + strlen(handle->root);
- if(alpm_list_find_str(alpm_pkg_get_files(dbpkg),fi lestr)) {
+ char *pkgfilestr = rpath + strlen(handle->root);
+ if(alpm_list_find_str(alpm_pkg_get_files(dbpkg), pkgfilestr)) {
resolved_conflict = 1;
}
free(rpath);
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index ee95c89..1d5eb0c 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -259,12 +259,12 @@ pmpkg_t SYMEXPORT *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstring)
* Dependencies can include versions with depmod operators.
* @param pkglist the list of local packages
* @param reversedeps handles the backward dependencies
- * @param remove an alpm_list_t* of packages to be removed
+ * @param remove_pkgs an alpm_list_t* of packages to be removed
* @param upgrade an alpm_list_t* of packages to be upgraded (remove-then-upgrade)
* @return an alpm_list_t* of pmpkg_t* of pmdepmissing_t pointers.
*/
alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_list_t *pkglist, int reversedeps,
- alpm_list_t *remove, alpm_list_t *upgrade)
+ alpm_list_t *remove_pkgs, alpm_list_t *upgrade)
{
alpm_list_t *i, *j;
alpm_list_t *targets, *dblist = NULL, *modified = NULL;
@@ -273,7 +273,7 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_list_t *pkglist, int reversedeps,

ALPM_LOG_FUNC;

- targets = alpm_list_join(alpm_list_copy(remove), alpm_list_copy(upgrade));
+ targets = alpm_list_join(alpm_list_copy(remove_pkgs), alpm_list_copy(upgrade));
for(i = pkglist; i; i = i->next) {
pmpkg_t *pkg = i->data;
if(_alpm_pkg_find(targets, pkg->name)) {
@@ -621,14 +621,14 @@ static pmpkg_t *resolvedep(pmdepend_t *dep, alpm_list_t *dbs,
count = alpm_list_count(providers);
if (count >= 1) {
/* default to first provider if there is no QUESTION callback */
- int index = 0;
+ int pkg_index = 0;
if(count > 1) {
/* if there is more than one provider, we ask the user */
QUESTION(handle->trans, PM_TRANS_CONV_SELECT_PROVIDER,
- providers, dep, NULL, &index);
+ providers, dep, NULL, &pkg_index);
}
- if(index >= 0 && index < count) {
- pmpkg_t *pkg = alpm_list_getdata(alpm_list_nth(providers, index));
+ if(pkg_index >= 0 && pkg_index < count) {
+ pmpkg_t *pkg = alpm_list_getdata(alpm_list_nth(providers, pkg_index));
alpm_list_free(providers);
return pkg;
}
@@ -688,7 +688,7 @@ pmpkg_t SYMEXPORT *alpm_find_dbs_satisfier(alpm_list_t *dbs, const char *depstri
*/
int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pkg,
alpm_list_t *preferred, alpm_list_t **packages,
- alpm_list_t *remove, alpm_list_t **data)
+ alpm_list_t *remove_pkgs, alpm_list_t **data)
{
int ret = 0;
alpm_list_t *i, *j;
@@ -713,7 +713,7 @@ int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pk
for(i = alpm_list_last(*packages); i; i = i->next) {
pmpkg_t *tpkg = i->data;
targ = alpm_list_add(NULL, tpkg);
- deps = alpm_checkdeps(localpkgs, 0, remove, targ);
+ deps = alpm_checkdeps(localpkgs, 0, remove_pkgs, targ);
alpm_list_free(targ);

for(j = deps; j; j = j->next) {
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c
index 983a3ac..8141d3e 100644
--- a/lib/libalpm/diskspace.c
+++ b/lib/libalpm/diskspace.c
@@ -262,7 +262,7 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local)
alpm_list_t *mount_points, *i;
alpm_mountpoint_t *root_mp;
size_t replaces = 0, current = 0, numtargs;
- int abort = 0;
+ int should_abort = 0;
alpm_list_t *targ;

numtargs = alpm_list_count(trans->add);
@@ -322,7 +322,7 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local)
if(data->used && data->read_only) {
_alpm_log(PM_LOG_ERROR, _("Partition %s is mounted read only
"),
data->mount_dir);
- abort = 1;
+ should_abort = 1;
} else if(data->used & USED_INSTALL) {
/* cushion is roughly min(5% capacity, 20MiB) */
long fivepc = ((long)data->fsp.f_blocks / 20) + 1;
@@ -337,7 +337,7 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local)
_alpm_log(PM_LOG_ERROR, _("Partition %s too full: %ld blocks needed, %ld blocks free
"),
data->mount_dir, data->max_blocks_needed + cushion,
(unsigned long)data->fsp.f_bfree);
- abort = 1;
+ should_abort = 1;
}
}
}
@@ -348,7 +348,7 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local)
}
FREELIST(mount_points);

- if(abort) {
+ if(should_abort) {
RET_ERR(PM_ERR_DISK_SPACE, -1);
}

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index d9e9488..a8d7ec0 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -142,12 +142,12 @@ static int curl_gethost(const char *url, char *buffer)
return 0;
}

-static int utimes_long(const char *path, long time)
+static int utimes_long(const char *path, long epoch)
{
- if(time != -1) {
+ if(epoch != -1) {
struct timeval tv[2];
memset(&tv, 0, sizeof(tv));
- tv[0].tv_sec = tv[1].tv_sec = time;
+ tv[0].tv_sec = tv[1].tv_sec = epoch;
return utimes(path, tv);
}
return 0;
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index fd40f19..0a5d031 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -44,53 +44,53 @@ pmhandle_t *handle = NULL;

pmhandle_t *_alpm_handle_new()
{
- pmhandle_t *handle;
+ pmhandle_t *new_handle;

ALPM_LOG_FUNC;

- CALLOC(handle, 1, sizeof(pmhandle_t), RET_ERR(PM_ERR_MEMORY, NULL));
+ CALLOC(new_handle, 1, sizeof(pmhandle_t), RET_ERR(PM_ERR_MEMORY, NULL));

- return handle;
+ return new_handle;
}

-void _alpm_handle_free(pmhandle_t *handle)
+void _alpm_handle_free(pmhandle_t *local_handle)
{
ALPM_LOG_FUNC;

- if(handle == NULL) {
+ if(local_handle == NULL) {
return;
}

/* close logfile */
- if(handle->logstream) {
- fclose(handle->logstream);
- handle->logstream= NULL;
+ if(local_handle->logstream) {
+ fclose(local_handle->logstream);
+ local_handle->logstream= NULL;
}
- if(handle->usesyslog) {
- handle->usesyslog = 0;
+ if(local_handle->usesyslog) {
+ local_handle->usesyslog = 0;
closelog();
}

#ifdef HAVE_LIBCURL
/* release curl handle */
- curl_easy_cleanup(handle->curl);
+ curl_easy_cleanup(local_handle->curl);
#endif

/* free memory */
- _alpm_trans_free(handle->trans);
- FREE(handle->root);
- FREE(handle->dbpath);
- FREELIST(handle->cachedirs);
- FREE(handle->logfile);
- FREE(handle->lockfile);
- FREE(handle->arch);
- FREE(handle->signaturedir);
- FREELIST(handle->dbs_sync);
- FREELIST(handle->noupgrade);
- FREELIST(handle->noextract);
- FREELIST(handle->ignorepkg);
- FREELIST(handle->ignoregrp);
- FREE(handle);
+ _alpm_trans_free(local_handle->trans);
+ FREE(local_handle->root);
+ FREE(local_handle->dbpath);
+ FREELIST(local_handle->cachedirs);
+ FREE(local_handle->logfile);
+ FREE(local_handle->lockfile);
+ FREE(local_handle->arch);
+ FREE(local_handle->signaturedir);
+ FREELIST(local_handle->dbs_sync);
+ FREELIST(local_handle->noupgrade);
+ FREELIST(local_handle->noextract);
+ FREELIST(local_handle->ignorepkg);
+ FREELIST(local_handle->ignoregrp);
+ FREE(local_handle);

}

diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 69c2b85..3da34c3 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -377,9 +377,9 @@ static void find_requiredby(pmpkg_t *pkg, pmdb_t *db, alpm_list_t **reqs)
const alpm_list_t *i;
for(i = _alpm_db_get_pkgcache(db); i; i = i->next) {
pmpkg_t *cachepkg = i->data;
- alpm_list_t *i;
- for(i = alpm_pkg_get_depends(cachepkg); i; i = i->next) {
- if(_alpm_depcmp(pkg, i->data)) {
+ alpm_list_t *deps;
+ for(deps = alpm_pkg_get_depends(cachepkg); deps; deps = deps->next) {
+ if(_alpm_depcmp(pkg, deps->data)) {
const char *cachepkgname = cachepkg->name;
if(alpm_list_find_str(*reqs, cachepkgname) == NULL) {
*reqs = alpm_list_add(*reqs, strdup(cachepkgname));
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 5428e40..1ddc8e0 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -304,7 +304,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
alpm_list_t *deps = NULL;
alpm_list_t *unresolvable = NULL;
alpm_list_t *i, *j;
- alpm_list_t *remove = NULL;
+ alpm_list_t *remove_pkgs = NULL;
int ret = 0;

ALPM_LOG_FUNC;
@@ -328,7 +328,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
for(i = trans->add; i; i = i->next) {
pmpkg_t *spkg = i->data;
for(j = spkg->removes; j; j = j->next) {
- remove = alpm_list_add(remove, j->data);
+ remove_pkgs = alpm_list_add(remove_pkgs, j->data);
}
}

@@ -342,7 +342,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
for(i = trans->add; i; i = i->next) {
pmpkg_t *pkg = i->data;
if(_alpm_resolvedeps(localpkgs, dbs_sync, pkg, trans->add,
- &resolved, remove, data) == -1) {
+ &resolved, remove_pkgs, data) == -1) {
unresolvable = alpm_list_add(unresolvable, pkg);
}
/* Else, [resolved] now additionally contains [pkg] and all of its
@@ -407,7 +407,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync

for(i = deps; i; i = i->next) {
pmconflict_t *conflict = i->data;
- pmpkg_t *rsync, *sync, *sync1, *sync2;
+ pmpkg_t *rsync, *syncpkg, *sync1, *sync2;

/* have we already removed one of the conflicting targets? */
sync1 = _alpm_pkg_find(trans->add, conflict->package1);
@@ -424,10 +424,10 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
pmdepend_t *dep2 = _alpm_splitdep(conflict->package2);
if(_alpm_depcmp(sync1, dep2)) {
rsync = sync2;
- sync = sync1;
+ syncpkg = sync1;
} else if(_alpm_depcmp(sync2, dep1)) {
rsync = sync1;
- sync = sync2;
+ syncpkg = sync2;
} else {
_alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected
"));
pm_errno = PM_ERR_CONFLICTING_DEPS;
@@ -450,7 +450,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
/* Prints warning */
_alpm_log(PM_LOG_WARNING,
_("removing '%s' from target list because it conflicts with '%s'
"),
- rsync->name, sync->name);
+ rsync->name, syncpkg->name);
trans->add = alpm_list_remove(trans->add, rsync, _alpm_pkg_cmp, NULL);
_alpm_pkg_free_trans(rsync); /* rsync is not transaction target anymore */
continue;
@@ -483,7 +483,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
_alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'
",
conflict->package1, conflict->package2);

- pmpkg_t *sync = _alpm_pkg_find(trans->add, conflict->package1);
+ pmpkg_t *syncpkg = _alpm_pkg_find(trans->add, conflict->package1);
pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, conflict->package2);
int doremove = 0;
QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, conflict->package1,
@@ -491,7 +491,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
if(doremove) {
/* append to the removes list */
_alpm_log(PM_LOG_DEBUG, "electing '%s' for removal
", conflict->package2);
- sync->removes = alpm_list_add(sync->removes, local);
+ syncpkg->removes = alpm_list_add(syncpkg->removes, local);
} else { /* abort */
_alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected
"));
pm_errno = PM_ERR_CONFLICTING_DEPS;
@@ -550,7 +550,7 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync

cleanup:
alpm_list_free(unresolvable);
- alpm_list_free(remove);
+ alpm_list_free(remove_pkgs);

return ret;
}
@@ -718,12 +718,12 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)

/* group sync records by repository and download */
for(i = handle->dbs_sync; i; i = i->next) {
- pmdb_t *current = i->data;
+ pmdb_t *current_pkg = i->data;

for(j = trans->add; j; j = j->next) {
pmpkg_t *spkg = j->data;

- if(spkg->origin != PKG_FROM_FILE && current == spkg->origin_data.db) {
+ if(spkg->origin != PKG_FROM_FILE && current_pkg == spkg->origin_data.db) {
const char *fname = NULL;

fname = alpm_pkg_get_filename(spkg);
@@ -757,12 +757,12 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
}

if(files) {
- EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current->treename, NULL);
- errors = _alpm_download_files(files, current->servers, cachedir);
+ EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current_pkg->treename, NULL);
+ errors = _alpm_download_files(files, current_pkg->servers, cachedir);

if (errors) {
_alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s
"),
- current->treename);
+ current_pkg->treename);
if(pm_errno == 0) {
pm_errno = PM_ERR_RETRIEVE;
}
@@ -785,7 +785,6 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)

/* if we have deltas to work with */
if(handle->usedelta && deltas) {
- int ret = 0;
errors = 0;
/* Check integrity of deltas */
EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
@@ -850,9 +849,9 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
pmdb_t *sdb = alpm_pkg_get_db(spkg);

if(sdb->pgp_verify != PM_PGP_VERIFY_NEVER) {
- int ret = _alpm_gpgme_checksig(filepath, pgpsig);
- if((sdb->pgp_verify == PM_PGP_VERIFY_ALWAYS && ret != 0) ||
- (sdb->pgp_verify == PM_PGP_VERIFY_OPTIONAL && ret == 1)) {
+ int gpg_ret = _alpm_gpgme_checksig(filepath, pgpsig);
+ if((sdb->pgp_verify == PM_PGP_VERIFY_ALWAYS && gpg_ret != 0) ||
+ (sdb->pgp_verify == PM_PGP_VERIFY_OPTIONAL && gpg_ret == 1)) {
errors++;
*data = alpm_list_add(*data, strdup(filename));
FREE(filepath);
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index 8125419..0390732 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -53,13 +53,13 @@
*/

/* Create a lock file */
-static int make_lock(pmhandle_t *handle)
+static int make_lock(pmhandle_t *local_handle)
{
int fd;
char *dir, *ptr;

/* create the dir of the lockfile first */
- dir = strdup(handle->lockfile);
+ dir = strdup(local_handle->lockfile);
ptr = strrchr(dir, '/');
if(ptr) {
*ptr = '';
@@ -71,27 +71,27 @@ static int make_lock(pmhandle_t *handle)
FREE(dir);

do {
- fd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000);
+ fd = open(local_handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000);
} while (fd == -1 && errno == EINTR);
if(fd > 0) {
FILE *f = fdopen(fd, "w");
fprintf(f, "%ld
", (long)getpid());
fflush(f);
fsync(fd);
- handle->lckstream = f;
+ local_handle->lckstream = f;
return 0;
}
return -1;
}

/* Remove a lock file */
-static int remove_lock(pmhandle_t *handle)
+static int remove_lock(pmhandle_t *local_handle)
{
- if(handle->lckstream != NULL) {
- fclose(handle->lckstream);
- handle->lckstream = NULL;
+ if(local_handle->lckstream != NULL) {
+ fclose(local_handle->lckstream);
+ local_handle->lckstream = NULL;
}
- if(unlink(handle->lockfile) == -1 && errno != ENOENT) {
+ if(unlink(local_handle->lockfile) == -1 && errno != ENOENT) {
return -1;
}
return 0;
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 660d4eb..86e31b6 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -298,13 +298,13 @@ int _alpm_unpack(const char *archive, const char *prefix, alpm_list_t *list, int

/* If specific files were requested, skip entries that don't match. */
if(list) {
- char *prefix = strdup(entryname);
- char *p = strstr(prefix,"/");
+ char *entry_prefix = strdup(entryname);
+ char *p = strstr(entry_prefix,"/");
if(p) {
*(p+1) = '';
}
- char *found = alpm_list_find_str(list, prefix);
- free(prefix);
+ char *found = alpm_list_find_str(list, entry_prefix);
+ free(entry_prefix);
if(!found) {
if (archive_read_data_skip(_archive) != ARCHIVE_OK) {
ret = 1;
@@ -485,22 +485,22 @@ int _alpm_run_chroot(const char *root, const char *path, char *const argv[])
} else {
/* this code runs for the parent only (wait on the child) */
int status;
- FILE *pipe;
+ FILE *pipefh;

close(pipefd[1]);
- pipe = fdopen(pipefd[0], "r");
- if(pipe == NULL) {
+ pipefh = fdopen(pipefd[0], "r");
+ if(pipefh == NULL) {
close(pipefd[0]);
retval = 1;
} else {
- while(!feof(pipe)) {
+ while(!feof(pipefh)) {
char line[PATH_MAX];
- if(fgets(line, PATH_MAX, pipe) == NULL)
+ if(fgets(line, PATH_MAX, pipefh) == NULL)
break;
alpm_logaction("%s", line);
EVENT(handle->trans, PM_TRANS_EVT_SCRIPTLET_INFO, line, NULL);
}
- fclose(pipe);
+ fclose(pipefh);
}

while(waitpid(pid, &status, 0) == -1) {
diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
index 333b819..25e8fae 100644
--- a/src/pacman/Makefile.am
+++ b/src/pacman/Makefile.am
@@ -17,7 +17,7 @@ DEFS = -DLOCALEDIR="@localedir@"
@DEFS@
INCLUDES = -I$(top_srcdir)/lib/libalpm

-AM_CFLAGS = -pedantic -D_GNU_SOURCE
+AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE

if USE_GIT_VERSION
GIT_VERSION := $(shell sh -c 'git describe --abbrev=4 --dirty | sed s/^v//')
diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 08c1cf3..8e52eac 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -614,14 +614,14 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
/* if padwid is < 0, we need to trim the string so padwid = 0 */
if(padwid < 0) {
int i = filenamelen - 3;
- wchar_t *p = wcfname;
+ wchar_t *ptr = wcfname;
/* grab the max number of char columns we can fill */
- while(i > 0 && wcwidth(*p) < i) {
- i -= wcwidth(*p);
- p++;
+ while(i > 0 && wcwidth(*ptr) < i) {
+ i -= wcwidth(*ptr);
+ ptr++;
}
/* then add the ellipsis and fill out any extra padding */
- wcscpy(p, L"...");
+ wcscpy(ptr, L"...");
padwid = i;

}
diff --git a/src/pacman/query.c b/src/pacman/query.c
index d2bfe69..2bc3373 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -286,7 +286,6 @@ static int query_search(alpm_list_t *targets)
static int query_group(alpm_list_t *targets)
{
alpm_list_t *i, *j;
- char *grpname = NULL;
int ret = 0;
pmdb_t *db_local = alpm_option_get_localdb();

@@ -305,6 +304,7 @@ static int query_group(alpm_list_t *targets)
}
} else {
for(i = targets; i; i = alpm_list_next(i)) {
+ char *grpname;
pmgrp_t *grp;
grpname = alpm_list_getdata(i);
grp = alpm_db_readgrp(db_local, grpname);
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 0b34f04..ea73668 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -488,7 +488,7 @@ static int sync_info(alpm_list_t *syncs, alpm_list_t *targets)
pkgstr = target;

for(j = syncs; j; j = alpm_list_next(j)) {
- pmdb_t *db = alpm_list_getdata(j);
+ db = alpm_list_getdata(j);

for(k = alpm_db_get_pkgcache(db); k; k = alpm_list_next(k)) {
pmpkg_t *pkg = alpm_list_getdata(k);
@@ -761,7 +761,6 @@ static int sync_trans(alpm_list_t *targets)
pm_fprintf(stderr, PM_LOG_ERROR, _("failed to prepare transaction (%s)
"),
alpm_strerrorlast());
switch(pm_errno) {
- alpm_list_t *i;
case PM_ERR_PKG_INVALID_ARCH:
for(i = data; i; i = alpm_list_next(i)) {
char *pkg = alpm_list_getdata(i);
@@ -830,7 +829,6 @@ static int sync_trans(alpm_list_t *targets)
pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)
"),
alpm_strerrorlast());
switch(pm_errno) {
- alpm_list_t *i;
case PM_ERR_FILE_CONFLICTS:
for(i = data; i; i = alpm_list_next(i)) {
pmfileconflict_t *conflict = alpm_list_getdata(i);
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
index 5b89400..28491b4 100644
--- a/src/pacman/upgrade.c
+++ b/src/pacman/upgrade.c
@@ -166,7 +166,6 @@ int pacman_upgrade(alpm_list_t *targets)
pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)
"),
alpm_strerrorlast());
switch(pm_errno) {
- alpm_list_t *i;
case PM_ERR_FILE_CONFLICTS:
for(i = data; i; i = alpm_list_next(i)) {
pmfileconflict_t *conflict = alpm_list_getdata(i);
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 51bb052..1e53f80 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -398,24 +398,24 @@ alpm_list_t *strsplit(const char *str, const char splitchar)
{
alpm_list_t *list = NULL;
const char *prev = str;
- char *dup = NULL;
+ char *p = NULL;

while((str = strchr(str, splitchar))) {
- dup = strndup(prev, (size_t)(str - prev));
- if(dup == NULL) {
+ p = strndup(prev, (size_t)(str - prev));
+ if(p == NULL) {
return NULL;
}
- list = alpm_list_add(list, dup);
+ list = alpm_list_add(list, p);

str++;
prev = str;
}

- dup = strdup(prev);
- if(dup == NULL) {
+ p = strdup(prev);
+ if(p == NULL) {
return NULL;
}
- list = alpm_list_add(list, dup);
+ list = alpm_list_add(list, p);

return list;
}
--
1.7.4.2

On 03/04/11 20:49, Sebastian Nowicki wrote:

The following have been renamed to avoid shadowing:

* conflict.c
- filestr -> pkgfilestr
- remove -> remove_pkgs
* deps.c
- index -> pkg_index
- remove -> remove_pkgs
- time -> epoch
* diskspace.c
- abort -> should_abort
* handle.c
- handle -> local_handle / new_handle
* package.c
- i -> deps
* sync.c (alpm)
- current -> current_pkg
- remove -> remove_pkgs
- ret -> gpg_ret
- sync -> syncpkg
* trans.c
- handle -> local_handle
* util.c (alpm)
- pipe -> pipefh
- prefix -> entry_prefix
* callback.c
- p -> ptr
* util.c (pacman)
- dup -> p

In some instance duplicate declarations were removed.

Signed-off-by: Sebastian Nowicki<sebnow@gmail.com>
---
This is a follow up for an earlier patch I sent back in December.
Apologies for not following up sooner. The feedback was to split the
patch up. This is the first patch from the split.

The API for alpm_checkdeps is slightly modified (a parameter is
renamed). I'm not sure whether this is a backwards incompatible change
or not.

Sorry for the size, I don't think it can be split up much further
without it turning into a dozen patches.

This patch was generated against the master branch.

lib/libalpm/Makefile.am | 2 +-
lib/libalpm/alpm.h | 2 +-
lib/libalpm/conflict.c | 8 +++---
lib/libalpm/deps.c | 18 ++++++++--------
lib/libalpm/diskspace.c | 8 +++---
lib/libalpm/dload.c | 6 ++--
lib/libalpm/handle.c | 50 +++++++++++++++++++++++-----------------------
lib/libalpm/package.c | 6 ++--
lib/libalpm/sync.c | 37 ++++++++++++++++-----------------
lib/libalpm/trans.c | 18 ++++++++--------
lib/libalpm/util.c | 20 +++++++++---------
src/pacman/Makefile.am | 2 +-
src/pacman/callback.c | 10 ++++----
src/pacman/query.c | 2 +-
src/pacman/sync.c | 4 +--
src/pacman/upgrade.c | 1 -
src/pacman/util.c | 14 ++++++------
17 files changed, 102 insertions(+), 106 deletions(-)

diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
index fb224a5..21f7057 100644
--- a/lib/libalpm/Makefile.am
+++ b/lib/libalpm/Makefile.am
@@ -7,7 +7,7 @@ include_HEADERS = alpm_list.h alpm.h

DEFS = -DLOCALEDIR="@localedir@" @DEFS@

-AM_CFLAGS = -pedantic -D_GNU_SOURCE
+AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE


I have not gone through all of these changes... I think adding extra
warnings etc should be done in configure.ac and only when --enable-debug
is specified (as is currently done with stack protector, -Werror etc).


But do we really want to add all these warnings in there? I use all of
these:


-Wclobbered -Wempty-body -Wfloat-equal -Wignored-qualifiers
-Wmissing-declarations -Wmissing-parameter-type -Wmissing-prototypes
-Wold-style-declaration -Woverride-init -Wsign-compare
-Wstrict-prototypes -Wtype-limits -Wuninitialized


I'm not sure anybody wants that...

Allan

On Mon, Apr 4, 2011 at 7:26 AM, Allan McRae <allan@archlinux.org> wrote:

> On 03/04/11 20:49, Sebastian Nowicki wrote:
>
>> The following have been renamed to avoid shadowing:
>>
>> * conflict.c
>> - filestr -> pkgfilestr
>> - remove -> remove_pkgs
>> * deps.c
>> - index -> pkg_index
>> - remove -> remove_pkgs
>> - time -> epoch
>> * diskspace.c
>> - abort -> should_abort
>> * handle.c
>> - handle -> local_handle / new_handle
>> * package.c
>> - i -> deps
>> * sync.c (alpm)
>> - current -> current_pkg
>> - remove -> remove_pkgs
>> - ret -> gpg_ret
>> - sync -> syncpkg
>> * trans.c
>> - handle -> local_handle
>> * util.c (alpm)
>> - pipe -> pipefh
>> - prefix -> entry_prefix
>> * callback.c
>> - p -> ptr
>> * util.c (pacman)
>> - dup -> p
>>
>> In some instance duplicate declarations were removed.
>>
>> Signed-off-by: Sebastian Nowicki<sebnow@gmail.com>
>> ---
>> This is a follow up for an earlier patch I sent back in December.
>> Apologies for not following up sooner. The feedback was to split the
>> patch up. This is the first patch from the split.
>>
>> The API for alpm_checkdeps is slightly modified (a parameter is
>> renamed). I'm not sure whether this is a backwards incompatible change
>> or not.
>>
>> Sorry for the size, I don't think it can be split up much further
>> without it turning into a dozen patches.
>>
>> This patch was generated against the master branch.
>>
>> lib/libalpm/Makefile.am | 2 +-
>> lib/libalpm/alpm.h | 2 +-
>> lib/libalpm/conflict.c | 8 +++---
>> lib/libalpm/deps.c | 18 ++++++++--------
>> lib/libalpm/diskspace.c | 8 +++---
>> lib/libalpm/dload.c | 6 ++--
>> lib/libalpm/handle.c | 50
>> +++++++++++++++++++++++-----------------------
>> lib/libalpm/package.c | 6 ++--
>> lib/libalpm/sync.c | 37 ++++++++++++++++-----------------
>> lib/libalpm/trans.c | 18 ++++++++--------
>> lib/libalpm/util.c | 20 +++++++++---------
>> src/pacman/Makefile.am | 2 +-
>> src/pacman/callback.c | 10 ++++----
>> src/pacman/query.c | 2 +-
>> src/pacman/sync.c | 4 +--
>> src/pacman/upgrade.c | 1 -
>> src/pacman/util.c | 14 ++++++------
>> 17 files changed, 102 insertions(+), 106 deletions(-)
>>
>> diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
>> index fb224a5..21f7057 100644
>> --- a/lib/libalpm/Makefile.am
>> +++ b/lib/libalpm/Makefile.am
>> @@ -7,7 +7,7 @@ include_HEADERS = alpm_list.h alpm.h
>>
>> DEFS = -DLOCALEDIR="@localedir@" @DEFS@
>>
>> -AM_CFLAGS = -pedantic -D_GNU_SOURCE
>> +AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE
>>
>
> I have not gone through all of these changes... I think adding extra
> warnings etc should be done in configure.ac and only when --enable-debug
> is specified (as is currently done with stack protector, -Werror etc).
>
> But do we really want to add all these warnings in there? I use all of
> these:
>
> -Wclobbered -Wempty-body -Wfloat-equal -Wignored-qualifiers
> -Wmissing-declarations -Wmissing-parameter-type -Wmissing-prototypes
> -Wold-style-declaration -Woverride-init -Wsign-compare -Wstrict-prototypes
> -Wtype-limits -Wuninitialized
>
> I'm not sure anybody wants that...
>
> Allan
>
>
I wasn't going to add it initially, but thought this would prevent shadowing
sneaking back in again. I don't really mind either way though.

Allan McRae wrote:
>
> But do we really want to add all these warnings in there? I use
> all of these:
>
> -Wclobbered -Wempty-body -Wfloat-equal -Wignored-qualifiers
> -Wmissing-declarations -Wmissing-parameter-type -Wmissing-prototypes
> -Wold-style-declaration -Woverride-init -Wsign-compare
> -Wstrict-prototypes -Wtype-limits -Wuninitialized
>
> I'm not sure anybody wants that...
>

If they pass (or close to), why not ? Developers should follow the same rules to
not break something that was respected before and to keep a consistent codebase.

On Sun, Apr 3, 2011 at 6:26 PM, Allan McRae <allan@archlinux.org> wrote:
> On 03/04/11 20:49, Sebastian Nowicki wrote:
>>
>> The following have been renamed to avoid shadowing:
>>
>> * * * ** conflict.c
>> * * * * * * * *- filestr -> *pkgfilestr
>> * * * * * * * *- remove -> *remove_pkgs
>> * * * ** deps.c
>> * * * * * * * *- index -> *pkg_index
>> * * * * * * * *- remove -> *remove_pkgs
>> * * * * * * * *- time -> *epoch
>> * * * ** diskspace.c
>> * * * * * * * *- abort -> *should_abort
>> * * * ** handle.c
>> * * * * * * * *- handle -> *local_handle / new_handle
>> * * * ** package.c
>> * * * * * *- i -> *deps
>> * * * ** sync.c (alpm)
>> * * * * * * * *- current -> *current_pkg
>> * * * * * * * *- remove -> *remove_pkgs
>> * * * * * * * *- ret -> *gpg_ret
>> * * * * * * * *- sync -> *syncpkg
>> * * * ** trans.c
>> * * * * * * * *- handle -> *local_handle
>> * * * ** util.c (alpm)
>> * * * * * * * *- pipe -> *pipefh
>> * * * * * * * *- prefix -> *entry_prefix
>> * * * ** callback.c
>> * * * * * *- p -> *ptr
>> * * * ** util.c (pacman)
>> * * * * * * * *- dup -> *p
>>
>> In some instance duplicate declarations were removed.
>>
>> Signed-off-by: Sebastian Nowicki<sebnow@gmail.com>
>> ---
>>
>> diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
>> index fb224a5..21f7057 100644
>> --- a/lib/libalpm/Makefile.am
>> +++ b/lib/libalpm/Makefile.am
>> @@ -7,7 +7,7 @@ include_HEADERS = alpm_list.h alpm.h
>>
>> *DEFS = -DLOCALEDIR="@localedir@" @DEFS@
>>
>> -AM_CFLAGS = -pedantic -D_GNU_SOURCE
>> +AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE
>
> I have not gone through all of these changes... * I think adding extra
> warnings etc should be done in configure.ac and only when --enable-debug is
> specified *(as is currently done with stack protector, -Werror etc).
Yes, this is the only place they belong, definitely not in the
Makefile (and only in the libalpm Makefile, why?).

> But do we really want to add all these warnings in there? * I use all of
> these:
>
> -Wclobbered -Wempty-body -Wfloat-equal -Wignored-qualifiers
> -Wmissing-declarations -Wmissing-parameter-type -Wmissing-prototypes
> -Wold-style-declaration -Woverride-init -Wsign-compare -Wstrict-prototypes
> -Wtype-limits -Wuninitialized
>
> I'm not sure anybody wants that...

I'm not against it, provided:
1. When it is checked in, the compile doesn't break.
2. When someone uses "exotic" compiliers such as icc, gcc 3.x,
whatever Cygwin has, and clang, we don't blow up or make the output
that much worse for them.

-Dan

On Tue, Apr 5, 2011 at 1:37 PM, Dan McGee <dpmcgee@gmail.com> wrote:

> On Sun, Apr 3, 2011 at 6:26 PM, Allan McRae <allan@archlinux.org> wrote:
> > On 03/04/11 20:49, Sebastian Nowicki wrote:
> >> diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
> >> index fb224a5..21f7057 100644
> >> --- a/lib/libalpm/Makefile.am
> >> +++ b/lib/libalpm/Makefile.am
> >> @@ -7,7 +7,7 @@ include_HEADERS = alpm_list.h alpm.h
> >>
> >> DEFS = -DLOCALEDIR="@localedir@" @DEFS@
> >>
> >> -AM_CFLAGS = -pedantic -D_GNU_SOURCE
> >> +AM_CFLAGS = -pedantic -Wshadow -D_GNU_SOURCE
> >
> > I have not gone through all of these changes... I think adding extra
> > warnings etc should be done in configure.ac and only when --enable-debug
> is
> > specified (as is currently done with stack protector, -Werror etc).
> Yes, this is the only place they belong, definitely not in the
> Makefile (and only in the libalpm Makefile, why?).


It was added to the pacman Makefile as well (later down in the patch),
wasn't really sure where to put it since I couldn't find other warning flags
(aside from -Wall).

I guess I'll just put it in the configure.ac script for now. It can always
be removed later if it causes issues. Might even check if icc produces
different results.