[pacman-dev] [PATCH 1/2] Fix warnings not usually emitted (CFLAGS not in configure.ac)

-Wbad-function-cast: * casting long int to pmpkgreason_t unecessarily * casting pmpkgreason_t to long unecessarily * casting off_t (signed integral) to float before division unecessarily -Wshadow: * shadowing "handle" variables renamed to "new_handle" and "local_handle" shadowing "filestr" renamed to "pkgfilestr" * shadowing "remove" renamed to "remove_pkgs" * shadowing "sync" renamed to "syncpkg" * Removed redundant declarations * shadowing "prefix" renamed to "entry_prefix" * shadowing "pipe" renamed to "pipe_handle" -Wconversion: * explicitely cast nread to size_t from ssize_t in dload.c (guaranteed not to be negative at this point) * use size_t, as opposed to int, for string length and byte sizes * use signed int in alpm_list_count as advertised by API (should probably be changed to usngiend int or size_t, but requires an API change) Signed-off-by: Sebastian Nowicki <sebnow@gmail.com> --- Some of these warnings are quite pedantic and wouldn't really cause issues, so they might not be desired. The change in alpm_list_count is backwards in my opinion - the API should be changed to use unsigned int (or size_t). I didn't want to change the API though. This doesn't fix all the warnings. There are some where I had doubts about the semantics of the code, and some where the warnings would effectively just be suppressed (mostly regarding casting off_t to double/float for printing file sizes). lib/libalpm/alpm_list.c | 2 +- lib/libalpm/be_files.c | 2 +- lib/libalpm/conflict.c | 8 +++--- lib/libalpm/db.c | 2 +- lib/libalpm/deps.c | 12 +++++----- lib/libalpm/dload.c | 2 +- lib/libalpm/handle.c | 48 +++++++++++++++++++++++----------------------- lib/libalpm/sync.c | 21 +++++++++---------- lib/libalpm/util.c | 24 +++++++++++----------- src/pacman/package.c | 4 +- src/pacman/query.c | 5 +-- src/pacman/sync.c | 4 +-- src/pacman/upgrade.c | 1 - 13 files changed, 65 insertions(+), 70 deletions(-) diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c index 80ba1ee..9afa950 100644 --- a/lib/libalpm/alpm_list.c +++ b/lib/libalpm/alpm_list.c @@ -576,7 +576,7 @@ void SYMEXPORT *alpm_list_getdata(const alpm_list_t *node) */ int SYMEXPORT alpm_list_count(const alpm_list_t *list) { - unsigned int i = 0; + int i = 0; const alpm_list_t *lp = list; while(lp) { ++i; diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 4432171..477b1d0 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -582,7 +582,7 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) if(fgets(line, sizeof(line), fp) == NULL) { goto error; } - info->reason = (pmpkgreason_t)atol(_alpm_strtrim(line)); + info->reason = atol(_alpm_strtrim(line)); } else if(strcmp(line, "%SIZE%") == 0 || strcmp(line, "%CSIZE%") == 0) { /* NOTE: the CSIZE and SIZE fields both share the "size" field * in the pkginfo_t struct. This can be done b/c CSIZE diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 694c38d..9d6616a 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -400,7 +400,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; int numtargs = alpm_list_count(upgrade); @@ -495,7 +495,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); @@ -540,8 +540,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/db.c b/lib/libalpm/db.c index c8a91a2..de297de 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -168,7 +168,7 @@ int SYMEXPORT alpm_db_setserver(pmdb_t *db, const char *url) alpm_list_t *i; int found = 0; char *newurl; - int len = 0; + size_t len = 0; ALPM_LOG_FUNC; diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index fd893a6..ff7af14 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -232,12 +232,12 @@ alpm_list_t SYMEXPORT *alpm_deptest(pmdb_t *db, alpm_list_t *targets) * 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 missing_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; @@ -246,7 +246,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) { void *pkg = i->data; if(alpm_list_find(targets, pkg, _alpm_pkg_cmp)) { @@ -585,7 +585,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, * searched first for any dependency packages needed to complete the * resolve, and to which will be added any [pkg] and all of its * dependencies not already on the list - * @param remove is the set of packages which will be removed in this + * @param remove_pkgs is the set of packages which will be removed in this * transaction * @param data returns the dependency which could not be satisfied in the * event of an error @@ -596,7 +596,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, */ 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) { alpm_list_t *i, *j; alpm_list_t *targ; @@ -620,7 +620,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) { pmdepmissing_t *miss = j->data; diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 9b59f52..7d5a904 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -250,7 +250,7 @@ static int download_internal(const char *url, const char *localpath, while((nread = fetchIO_read(dlf, buffer, PM_DLBUF_LEN)) > 0) { check_stop(); size_t nwritten = 0; - nwritten = fwrite(buffer, 1, nread, localf); + nwritten = fwrite(buffer, 1, (size_t)nread, localf); if((nwritten != (size_t)nread) || ferror(localf)) { pm_errno = PM_ERR_RETRIEVE; _alpm_log(PM_LOG_ERROR, _("error writing to file '%s': %s\n"), diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index aa34cf4..6434044 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -44,48 +44,48 @@ 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)); - handle->lckfd = -1; + CALLOC(new_handle, 1, sizeof(pmhandle_t), RET_ERR(PM_ERR_MEMORY, NULL)); + new_handle->lckfd = -1; - 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(); } /* 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); - 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); + FREELIST(local_handle->dbs_sync); + FREELIST(local_handle->noupgrade); + FREELIST(local_handle->noextract); + FREELIST(local_handle->ignorepkg); + FREELIST(local_handle->ignoregrp); + FREE(local_handle); } alpm_cb_log SYMEXPORT alpm_option_get_logcb() diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index f819396..fab67c9 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -411,7 +411,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; @@ -435,7 +435,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); } } @@ -447,7 +447,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 @@ -512,7 +512,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); @@ -529,10 +529,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; @@ -555,7 +555,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; @@ -588,7 +588,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, @@ -596,7 +596,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; @@ -651,7 +651,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); } @@ -894,7 +894,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); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 5bf4ef1..c74649e 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -338,13 +338,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; @@ -524,22 +524,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 *pipe_handle; close(pipefd[1]); - pipe = fdopen(pipefd[0], "r"); - if(pipe == NULL) { + pipe_handle = fdopen(pipefd[0], "r"); + if(pipe_handle == NULL) { close(pipefd[0]); retval = 1; } else { - while(!feof(pipe)) { + while(!feof(pipe_handle)) { char line[PATH_MAX]; - if(fgets(line, PATH_MAX, pipe) == NULL) + if(fgets(line, PATH_MAX, pipe_handle) == NULL) break; alpm_logaction("%s", line); EVENT(handle->trans, PM_TRANS_EVT_SCRIPTLET_INFO, line, NULL); } - fclose(pipe); + fclose(pipe_handle); } while(waitpid(pid, &status, 0) == -1) { @@ -668,7 +668,7 @@ int _alpm_lstat(const char *path, struct stat *buf) { int ret; char *newpath = strdup(path); - int len = strlen(newpath); + size_t len = strlen(newpath); /* strip the trailing slash if one exists */ if(len != 0 && newpath[len - 1] == '/') { @@ -781,7 +781,7 @@ char *_alpm_archive_fgets(char *line, size_t size, struct archive *a) char *i; for(i = line; i < last; i++) { - int ret = archive_read_data(a, i, 1); + ssize_t ret = archive_read_data(a, i, 1); /* special check for first read- if null, return null, * this indicates EOF */ if(i == line && (ret <= 0 || *i == '\0')) { diff --git a/src/pacman/package.c b/src/pacman/package.c index ac84a0c..2b1faae 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -67,7 +67,7 @@ void dump_pkg_full(pmpkg_t *pkg, int level) strftime(idatestr, 50, "%c", localtime(&idate)); } - switch((long)alpm_pkg_get_reason(pkg)) { + switch(alpm_pkg_get_reason(pkg)) { case PM_PKG_REASON_EXPLICIT: reason = _("Explicitly installed"); break; @@ -242,7 +242,7 @@ void dump_pkg_changelog(pmpkg_t *pkg) } else { /* allocate a buffer to get the changelog back in chunks */ char buf[CLBUF_SIZE]; - int ret = 0; + size_t ret = 0; while((ret = alpm_pkg_changelog_read(buf, CLBUF_SIZE, pkg, fp))) { if(ret < CLBUF_SIZE) { /* if we hit the end of the file, we need to add a null terminator */ diff --git a/src/pacman/query.c b/src/pacman/query.c index 5538e81..740da82 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -266,7 +266,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; if(targets == NULL) { for(j = alpm_db_get_grpcache(db_local); j; j = alpm_list_next(j)) { @@ -283,8 +282,8 @@ static int query_group(alpm_list_t *targets) } } else { for(i = targets; i; i = alpm_list_next(i)) { + char *grpname = alpm_list_getdata(i); pmgrp_t *grp; - grpname = alpm_list_getdata(i); grp = alpm_db_readgrp(db_local, grpname); if(grp) { const alpm_list_t *p, *packages = alpm_grp_get_pkgs(grp); @@ -369,7 +368,7 @@ static int check(pmpkg_t *pkg) { alpm_list_t *i; const char *root; - int allfiles = 0, errors = 0; + unsigned int allfiles = 0, errors = 0; size_t rootlen; char f[PATH_MAX]; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index b9497d6..8970b5d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -468,7 +468,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); @@ -642,7 +642,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); @@ -711,7 +710,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 c9c8301..fcd1295 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -155,7 +155,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); -- 1.7.2.2