Signed-off-by: Sebastian Nowicki --- lib/libalpm/package.c | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 0060300..717d32c 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -488,12 +488,15 @@ void SYMEXPORT *alpm_pkg_changelog_open(pmpkg_t *pkg) /** * Read data from an open changelog 'file stream'. Similar to fread in - * functionality, this function takes a buffer and amount of data to read. + * functionality, this function takes a buffer and amount of data to read. If an + * error occurs pm_errno will be set. + * * @param ptr a buffer to fill with raw changelog data * @param size the size of the buffer * @param pkg the package that the changelog is being read from * @param fp a 'file stream' to the package changelog - * @return the number of characters read, or 0 if there is no more data + * @return the number of characters read, or 0 if there is no more data or an + * error occurred. */ size_t SYMEXPORT alpm_pkg_changelog_read(void *ptr, size_t size, const pmpkg_t *pkg, const void *fp) @@ -502,7 +505,14 @@ size_t SYMEXPORT alpm_pkg_changelog_read(void *ptr, size_t size, if(pkg->origin == PKG_FROM_CACHE) { ret = fread(ptr, 1, size, (FILE*)fp); } else if(pkg->origin == PKG_FROM_FILE) { - ret = archive_read_data((struct archive*)fp, ptr, size); + ssize_t sret = archive_read_data((struct archive*)fp, ptr, size); + /* Report error (negative values) */ + if(sret < 0) { + pm_errno = PM_ERR_LIBARCHIVE; + ret = 0; + } else { + ret = (size_t)sret; + } } return(ret); } -- 1.7.2.2

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 --- 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\n", 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),filestr)) { + 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\n"), 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\n"), 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\n")); 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'\n"), - 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'\n", 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\n", 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\n")); 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\n"), - 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 = '\0'; @@ -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\n", (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) = '\0'; } - 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)\n"), 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)\n"), 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)\n"), 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 Mon, Apr 4, 2011 at 7:26 AM, Allan McRae 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 --- 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.

On Sun, Apr 3, 2011 at 6:26 PM, Allan McRae 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 ---

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