This is an automated email from the git hooks/post-receive script. guillem pushed a commit to branch master in repository dpkg.
commit 9f7e58acdf28043bce2dfaf24ba27bb878418658 Author: Guillem Jover <[email protected]> Date: Wed Mar 15 13:33:09 2017 +0100 Use internerr() or BUG() instead of assert() The problem with assert() is that it does not print the contents of the variables. It also can be disabled on NDEBUG. But we always want these consistency checks no matter what, and they are not performance sensitive anyway. Enable -Wno-nonnull-compare so that we can keep doing run-time non-NULL checks in functions, instead of just compile-time checks. --- TODO | 1 - debian/changelog | 3 +++ debian/rules | 7 ++++- debian/usertags | 3 ++- dpkg-split/join.c | 5 ++-- dselect/baselist.cc | 6 +++-- dselect/main.cc | 1 - dselect/methlist.cc | 11 +++++--- dselect/method.cc | 7 ++--- dselect/methparse.cc | 5 ++-- dselect/pkgdepcon.cc | 4 +-- dselect/pkglist.cc | 23 +++++++++++----- dselect/pkgsublist.cc | 9 ++++--- dselect/pkgtop.cc | 5 ++-- lib/dpkg/arch.c | 4 +-- lib/dpkg/dbmodify.c | 18 +++++++++---- lib/dpkg/dump.c | 68 +++++++++++++++++++++++++++++++---------------- lib/dpkg/file.c | 4 +-- lib/dpkg/parse.c | 6 +++-- lib/dpkg/path-remove.c | 4 +-- lib/dpkg/pkg-array.c | 5 ++-- lib/dpkg/pkg-db.c | 7 ++--- lib/dpkg/pkg.c | 6 +++-- lib/dpkg/triglib.c | 15 ++++++++--- m4/dpkg-compiler.m4 | 1 + src/archives.c | 19 +++++++++---- src/configure.c | 5 ++-- src/depcon.c | 13 +++++---- src/enquiry.c | 5 ++-- src/filesdb.c | 10 ++++--- src/packages.c | 23 +++++++++++----- src/script.c | 6 +++-- src/trigproc.c | 15 +++++++---- src/unpack.c | 18 +++++++++---- utils/start-stop-daemon.c | 8 +++--- 35 files changed, 235 insertions(+), 115 deletions(-) diff --git a/TODO b/TODO index bc2dee0..a4f485d 100644 --- a/TODO +++ b/TODO @@ -28,7 +28,6 @@ TODO - Do more unused header include removal. - Add needed includes to all header files. - Get rid of useless "unsigned" modifiers. - - Use internerr instead of assert, and print more meaningful messages. - Use enums for currently hardcoded literals (replacingfilesandsaid, filetriggers_edited, etc). - Do not use nfmalloc (and friends) for non in-core db memory. diff --git a/debian/changelog b/debian/changelog index 0574fb5..3d59b42 100644 --- a/debian/changelog +++ b/debian/changelog @@ -44,6 +44,9 @@ dpkg (1.19.1) UNRELEASED; urgency=medium initialized, instead of non-zero. * Switch a fatal() call in start-stop-daemon into the new BUG() macro, because it is really an internal error. + * Switch all assert() calls (except in update-alternatives) into internerr() + or BUG() calls, to get way better reporting with variable contents and + descriptions, and to make them always present independent of NDEBUG. * Architecture support: - Add support for riscv64 CPU. Closes: #822914 Thanks to Manuel A. Fernandez Montecelo <[email protected]> diff --git a/debian/rules b/debian/rules index 27a2499..d5e13c6 100755 --- a/debian/rules +++ b/debian/rules @@ -3,7 +3,12 @@ # Copyright © 2004 Scott James Remnant <[email protected]> # Copyright © 2006-2012 Guillem Jover <[email protected]> -WFLAGS := -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers +WFLAGS := \ + -Wall -Wextra \ + -Wno-missing-field-initializers \ + -Wno-nonnull-compare \ + -Wno-unused-parameter \ + $(nil) # Use the in-tree dpkg-buildflags dpkg_buildflags = \ diff --git a/debian/usertags b/debian/usertags index 5ef5a44..97a6d8d 100644 --- a/debian/usertags +++ b/debian/usertags @@ -15,7 +15,8 @@ Generic tags usertag description ------- ----------- -assert Errors caused by assert(). +assert Errors caused by assert() (obsoleted by internerr() + usage). segfault Errors that cause segmentation faults. conffile Issues with configuration file handling. symlink Issues related to symlink handling. diff --git a/dpkg-split/join.c b/dpkg-split/join.c index 0a61bc9..b5e1471 100644 --- a/dpkg-split/join.c +++ b/dpkg-split/join.c @@ -21,7 +21,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <limits.h> #include <string.h> #include <fcntl.h> @@ -120,7 +119,9 @@ do_join(const char *const *argv) refi= NULL; for (pq= queue; pq; pq= pq->nextinqueue) if (!refi || pq->info.thispartn < refi->thispartn) refi= &pq->info; - assert(refi); + if (refi == NULL) + internerr("empty deb part queue"); + partlist= nfmalloc(sizeof(struct partinfo*)*refi->maxpartn); for (i = 0; i < refi->maxpartn; i++) partlist[i] = NULL; diff --git a/dselect/baselist.cc b/dselect/baselist.cc index d74ed43..80fee69 100644 --- a/dselect/baselist.cc +++ b/dselect/baselist.cc @@ -25,7 +25,6 @@ #include <sys/ioctl.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <termios.h> @@ -155,7 +154,10 @@ baselist::draw_column_item(column &col, int y, const char *item) void baselist::setheights() { int y= ymax - (title_height + colheads_height + thisstate_height); - assert(y>=1); + + if (y < 1) + internerr("widget y=%d < 1", y); + if (showinfo==2 && y>=7) { list_height= 5; whatinfo_height= 1; diff --git a/dselect/main.cc b/dselect/main.cc index 55ced33..e634e74 100644 --- a/dselect/main.cc +++ b/dselect/main.cc @@ -26,7 +26,6 @@ #include <sys/types.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <limits.h> #if HAVE_LOCALE_H diff --git a/dselect/methlist.cc b/dselect/methlist.cc index 331bf05..6cc280c 100644 --- a/dselect/methlist.cc +++ b/dselect/methlist.cc @@ -22,7 +22,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <stdio.h> @@ -129,10 +128,16 @@ methodlist::methodlist() : baselist(&methodlistbindings) { struct dselect_option *opt, **ip; for (opt=options, ip=table, nitems=0; opt; opt=opt->next, nitems++) { - if (opt == coption) { assert(newcursor==-1); newcursor= nitems; } + if (opt == coption) { + if (newcursor != -1) + internerr("multiple methods with same index"); + newcursor = nitems; + } *ip++= opt; } - assert(nitems==noptions); + if (nitems != noptions) + internerr("inconsistent number of items: ntimes=%d != noptions=%d", + nitems, noptions); if (newcursor==-1) newcursor= 0; setcursor(newcursor); diff --git a/dselect/method.cc b/dselect/method.cc index 74ac984..76863bc 100644 --- a/dselect/method.cc +++ b/dselect/method.cc @@ -27,7 +27,6 @@ #include <sys/file.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <limits.h> #include <string.h> @@ -71,8 +70,10 @@ sthfailed(const char * reasoning) static void cu_unlockmethod(int, void**) { struct flock fl; - assert(methodlockfile); - assert(methlockfd >= 0); + if (methodlockfile == NULL) + internerr("method lock file is NULL"); + if (methlockfd < 0) + internerr("method lock fd is %d < 0", methlockfd); fl.l_type=F_UNLCK; fl.l_whence= SEEK_SET; fl.l_start=fl.l_len=0; if (fcntl(methlockfd,F_SETLK,&fl) == -1) sthfailed(_("cannot unlock access method area")); diff --git a/dselect/methparse.cc b/dselect/methparse.cc index 131e79f..a1ab31a 100644 --- a/dselect/methparse.cc +++ b/dselect/methparse.cc @@ -26,7 +26,6 @@ #include <sys/stat.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <limits.h> #include <string.h> @@ -292,7 +291,9 @@ void getcurrentopt() { void writecurrentopt() { struct atomic_file *file; - assert(methoptfile); + if (methoptfile == NULL) + internerr("method options filename is NULL"); + file = atomic_file_new(methoptfile, (enum atomic_file_flags)0); atomic_file_open(file); if (fprintf(file->fp, "%s %s\n", coption->meth->name, coption->name) == EOF) diff --git a/dselect/pkgdepcon.cc b/dselect/pkgdepcon.cc index 87886ff..2104d2c 100644 --- a/dselect/pkgdepcon.cc +++ b/dselect/pkgdepcon.cc @@ -22,7 +22,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> #include <stdio.h> @@ -401,7 +400,8 @@ packagelist::deppossatisfied(deppossi *possi, perpackagestate **fixbyupgrade) // been specified, in which case we don't need to look at the rest // anyway. if (useavailable(&possi->ed->pkg)) { - assert(want == PKG_WANT_INSTALL); + if (want != PKG_WANT_INSTALL) + internerr("depossi package is not want-install, is %d", want); return versionsatisfied(&possi->ed->pkg.available, possi); } else { if (versionsatisfied(&possi->ed->pkg.installed, possi)) diff --git a/dselect/pkglist.cc b/dselect/pkglist.cc index 105f3c1..cb144e7 100644 --- a/dselect/pkglist.cc +++ b/dselect/pkglist.cc @@ -23,7 +23,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <stdlib.h> @@ -113,7 +112,10 @@ void packagelist::addheading(enum ssavailval ssavail, pkgpriority priority, const char *otherpriority, const char *section) { - assert(nitems <= nallocated); + if (nitems > nallocated) + internerr("inconsistent state: ntimes=%d > nallocated=%d", + nitems, nallocated); + if (nitems == nallocated) { nallocated += nallocated+50; struct perpackagestate **newtable= new struct perpackagestate*[nallocated]; @@ -266,7 +268,9 @@ void packagelist::sortmakeheads() { discardheadings(); ensurestatsortinfo(); sortinplace(); - assert(nitems); + + if (nitems == 0) + internerr("cannot sort 0 items"); debug(dbg_general, "packagelist[%p]::sortmakeheads() sortorder=%d statsortorder=%d", @@ -275,7 +279,9 @@ void packagelist::sortmakeheads() { int nrealitems= nitems; addheading(ssa_none, sss_none, PKG_PRIO_UNSET, nullptr, nullptr); - assert(sortorder != so_unsorted); + if (sortorder == so_unsorted) + internerr("cannot sort unsorted order"); + if (sortorder == so_alpha && statsortorder == sso_unsorted) { sortinplace(); return; } // Important: do not save pointers into table in this function, because @@ -287,7 +293,8 @@ void packagelist::sortmakeheads() { int a; for (a=0; a<nrealitems; a++) { thispkg= table[a]->pkg; - assert(thispkg->set->name); + if (thispkg->set->name == NULL) + internerr("package set has no name at table index %d", a); int ssdiff= 0; ssavailval ssavail= ssa_none; ssstateval ssstate= sss_none; @@ -456,11 +463,13 @@ perpackagestate::free(bool recursive) if (pkg->set->name) { if (modstatdb_get_status() == msdbrw_write) { if (uprec) { - assert(recursive); + if (!recursive) + internerr("unexpected non-recursive free requested"); uprec->selected= selected; pkg->clientdata= uprec; } else { - assert(!recursive); + if (recursive) + internerr("unexpected recursive free requested"); if (pkg->want != selected && !(pkg->want == PKG_WANT_UNKNOWN && selected == PKG_WANT_PURGE)) { pkg->want= selected; diff --git a/dselect/pkgsublist.cc b/dselect/pkgsublist.cc index 30b202f..d25334f 100644 --- a/dselect/pkgsublist.cc +++ b/dselect/pkgsublist.cc @@ -22,7 +22,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> #include <stdio.h> @@ -102,8 +101,12 @@ void packagelist::addunavailable(deppossi *possi) { if (!recursive) return; if (alreadydone(&unavdone,possi)) return; - assert(possi->up->up->clientdata); - assert(possi->up->up->clientdata->uprec); + if (possi->up->up->clientdata == nullptr) + internerr("deppossi from package %s has nullptr clientdata", + pkg_name(possi->up->up, pnaw_always)); + if (possi->up->up->clientdata->uprec == nullptr) + internerr("deppossi from package %s has nullptr clientdata's uprec", + pkg_name(possi->up->up, pnaw_always)); varbuf& vb= possi->up->up->clientdata->relations; vb(possi->ed->name); diff --git a/dselect/pkgtop.cc b/dselect/pkgtop.cc index 0da1f10..6e233a1 100644 --- a/dselect/pkgtop.cc +++ b/dselect/pkgtop.cc @@ -22,7 +22,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> #include <stdio.h> @@ -42,7 +41,9 @@ pkgprioritystring(const struct pkginfo *pkg) } else if (pkg->priority == PKG_PRIO_OTHER) { return pkg->otherpriority; } else { - assert(pkg->priority <= PKG_PRIO_UNKNOWN); + if (pkg->priority > PKG_PRIO_UNKNOWN) + internerr("package %s has out-of-range priority %d", + pkg_name(pkg, pnaw_always), pkg->priority); return gettext(prioritystrings[pkg->priority]); } } diff --git a/lib/dpkg/arch.c b/lib/dpkg/arch.c index 0999571..36b4829 100644 --- a/lib/dpkg/arch.c +++ b/lib/dpkg/arch.c @@ -23,7 +23,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <limits.h> #include <string.h> #include <stdbool.h> @@ -59,7 +58,8 @@ dpkg_arch_name_is_illegal(const char *name) static char buf[150]; const char *p = name; - assert(name); + if (name == NULL) + internerr("arch name argument is NULL"); if (!*p) return _("may not be empty string"); if (!c_isalnum(*p)) diff --git a/lib/dpkg/dbmodify.c b/lib/dpkg/dbmodify.c index ec11021..9baa583 100644 --- a/lib/dpkg/dbmodify.c +++ b/lib/dpkg/dbmodify.c @@ -27,7 +27,6 @@ #include <sys/types.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <limits.h> #include <string.h> @@ -307,13 +306,19 @@ modstatdb_get_status(void) void modstatdb_checkpoint(void) { int i; - assert(cstatus >= msdbrw_write); + if (cstatus < msdbrw_write) + internerr("modstatdb status '%d' is not writtable", cstatus); + writedb(statusfile, wdb_must_sync); for (i=0; i<nextupdate; i++) { sprintf(updatefnrest, IMPORTANTFMT, i); + /* Have we made a real mess? */ - assert(strlen(updatefnrest) <= IMPORTANTMAXLEN); + if (strlen(updatefnrest) > IMPORTANTMAXLEN) + internerr("modstatdb update entry name '%s' longer than %d", + updatefnrest, IMPORTANTMAXLEN); + if (unlink(updatefnbuf)) ohshite(_("failed to remove my own update file %.255s"),updatefnbuf); } @@ -349,7 +354,8 @@ void modstatdb_shutdown(void) { static void modstatdb_note_core(struct pkginfo *pkg) { - assert(cstatus >= msdbrw_write); + if (cstatus < msdbrw_write) + internerr("modstatdb status '%d' is not writtable", cstatus); varbuf_reset(&uvb); varbufrecord(&uvb, pkg, &pkg->installed); @@ -377,7 +383,9 @@ modstatdb_note_core(struct pkginfo *pkg) dir_sync_path(updatesdir); /* Have we made a real mess? */ - assert(strlen(updatefnrest) <= IMPORTANTMAXLEN); + if (strlen(updatefnrest) > IMPORTANTMAXLEN) + internerr("modstatdb update entry name '%s' longer than %d", + updatefnrest, IMPORTANTMAXLEN); nextupdate++; diff --git a/lib/dpkg/dump.c b/lib/dpkg/dump.c index a04f673..8086cf7 100644 --- a/lib/dpkg/dump.c +++ b/lib/dpkg/dump.c @@ -30,7 +30,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <unistd.h> @@ -57,7 +56,9 @@ w_name(struct varbuf *vb, const struct pkginfo *pkg, const struct pkgbin *pkgbin, enum fwriteflags flags, const struct fieldinfo *fip) { - assert(pkg->set->name); + if (pkg->set->name == NULL) + internerr("pkgset has no name"); + if (flags&fw_printheader) varbuf_add_str(vb, "Package: "); varbuf_add_str(vb, pkg->set->name); @@ -230,7 +231,11 @@ w_priority(struct varbuf *vb, { if (pkg->priority == PKG_PRIO_UNKNOWN) return; - assert(pkg->priority <= PKG_PRIO_UNKNOWN); + + if (pkg->priority > PKG_PRIO_UNKNOWN) + internerr("package %s has out-of-range priority %d", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg->priority); + if (flags&fw_printheader) varbuf_add_str(vb, "Priority: "); varbuf_add_str(vb, pkg_priority_name(pkg)); @@ -245,38 +250,46 @@ w_status(struct varbuf *vb, { if (pkgbin != &pkg->installed) return; - assert(pkg->want <= PKG_WANT_PURGE); - assert(pkg->eflag <= PKG_EFLAG_REINSTREQ); -#define PEND pkg->trigpend_head -#define AW pkg->trigaw.head + if (pkg->want > PKG_WANT_PURGE) + internerr("package %s has unknown want state %d", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg->want); + if (pkg->eflag > PKG_EFLAG_REINSTREQ) + internerr("package %s has unknown error state %d", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg->eflag); + switch (pkg->status) { case PKG_STAT_NOTINSTALLED: case PKG_STAT_CONFIGFILES: - assert(!PEND); - assert(!AW); + if (pkg->trigpend_head || pkg->trigaw.head) + internerr("package %s in state %s, has awaited or pending triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); break; case PKG_STAT_HALFINSTALLED: case PKG_STAT_UNPACKED: case PKG_STAT_HALFCONFIGURED: - assert(!PEND); + if (pkg->trigpend_head) + internerr("package %s in state %s, has pending triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); break; case PKG_STAT_TRIGGERSAWAITED: - assert(AW); + if (pkg->trigaw.head == NULL) + internerr("package %s in state %s, has no awaited triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); break; case PKG_STAT_TRIGGERSPENDING: - assert(PEND); - assert(!AW); + if (pkg->trigpend_head == NULL || pkg->trigaw.head) + internerr("package %s in stata %s, has awaited or no pending triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); break; case PKG_STAT_INSTALLED: - assert(!PEND); - assert(!AW); + if (pkg->trigpend_head || pkg->trigaw.head) + internerr("package %s in state %s, has awaited or pending triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); break; default: internerr("unknown package status '%d'", pkg->status); } -#undef PEND -#undef AW if (flags&fw_printheader) varbuf_add_str(vb, "Status: "); @@ -295,7 +308,9 @@ void varbufdependency(struct varbuf *vb, struct dependency *dep) { possdel= ""; for (dop= dep->list; dop; dop= dop->next) { - assert(dop->up == dep); + if (dop->up != dep) + internerr("dependency and deppossi not linked properly"); + varbuf_add_str(vb, possdel); possdel = " | "; varbuf_add_str(vb, dop->ed->name); @@ -339,7 +354,10 @@ w_dependency(struct varbuf *vb, for (dyp = pkgbin->depends; dyp; dyp = dyp->next) { if (dyp->type != fip->integer) continue; - assert(dyp->up == pkg); + + if (dyp->up != pkg) + internerr("dependency and package %s not linked properly", + pkgbin_name(pkg, pkgbin, pnaw_always)); if (dep_found) { varbuf_add_str(vb, ", "); @@ -389,8 +407,10 @@ w_trigpend(struct varbuf *vb, if (pkgbin == &pkg->available || !pkg->trigpend_head) return; - assert(pkg->status >= PKG_STAT_TRIGGERSAWAITED && - pkg->status <= PKG_STAT_TRIGGERSPENDING); + if (pkg->status < PKG_STAT_TRIGGERSAWAITED || + pkg->status > PKG_STAT_TRIGGERSPENDING) + internerr("package %s in non-trigger state %s, has pending triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); if (flags & fw_printheader) varbuf_add_str(vb, "Triggers-Pending:"); @@ -412,8 +432,10 @@ w_trigaw(struct varbuf *vb, if (pkgbin == &pkg->available || !pkg->trigaw.head) return; - assert(pkg->status > PKG_STAT_CONFIGFILES && - pkg->status <= PKG_STAT_TRIGGERSAWAITED); + if (pkg->status <= PKG_STAT_CONFIGFILES || + pkg->status > PKG_STAT_TRIGGERSAWAITED) + internerr("package %s in state %s, has awaited triggers", + pkgbin_name(pkg, pkgbin, pnaw_always), pkg_status_name(pkg)); if (flags & fw_printheader) varbuf_add_str(vb, "Triggers-Awaited:"); diff --git a/lib/dpkg/file.c b/lib/dpkg/file.c index f8afdeb..cd26122 100644 --- a/lib/dpkg/file.c +++ b/lib/dpkg/file.c @@ -25,7 +25,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <fcntl.h> #include <unistd.h> @@ -79,7 +78,8 @@ file_unlock(int lockfd, const char *lock_desc) { struct flock fl; - assert(lockfd >= 0); + if (lockfd < 0) + internerr("lock fd is %d < 0", lockfd); file_lock_setup(&fl, F_UNLCK); diff --git a/lib/dpkg/parse.c b/lib/dpkg/parse.c index d75c544..2210678 100644 --- a/lib/dpkg/parse.c +++ b/lib/dpkg/parse.c @@ -28,7 +28,6 @@ #include <sys/mman.h> #endif -#include <assert.h> #include <fcntl.h> #include <string.h> #include <unistd.h> @@ -493,7 +492,10 @@ pkg_parse_copy(struct parsedb_state *ps, dst_pkg->trigpend_head = src_pkg->trigpend_head; dst_pkg->trigaw = src_pkg->trigaw; for (ta = dst_pkg->trigaw.head; ta; ta = ta->sameaw.next) { - assert(ta->aw == src_pkg); + if (ta->aw != src_pkg) + internerr("trigger awaited package %s and origin package %s not linked properly", + pkg_name(ta->aw, pnaw_always), + pkgbin_name(src_pkg, src_pkgbin, pnaw_always)); ta->aw = dst_pkg; /* ->othertrigaw_head is updated by trig_note_aw in *(pkg_db_find()) * rather than in dst_pkg. */ diff --git a/lib/dpkg/path-remove.c b/lib/dpkg/path-remove.c index ab26b4a..ef9cde7 100644 --- a/lib/dpkg/path-remove.c +++ b/lib/dpkg/path-remove.c @@ -24,7 +24,6 @@ #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <unistd.h> @@ -122,7 +121,8 @@ path_remove_tree(const char *pathname) const char *u; u = path_skip_slash_dotslash(pathname); - assert(*u); + if (u[0] == '\0') + internerr("pathname '%s' reduces to nothing", pathname); debug(dbg_eachfile, "%s '%s'", __func__, pathname); if (!rmdir(pathname)) diff --git a/lib/dpkg/pkg-array.c b/lib/dpkg/pkg-array.c index 24c6dc0..cf5c34d 100644 --- a/lib/dpkg/pkg-array.c +++ b/lib/dpkg/pkg-array.c @@ -22,7 +22,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> #include <stdlib.h> @@ -74,7 +73,9 @@ pkg_array_init_from_db(struct pkg_array *a) a->pkgs[i] = pkg; pkg_db_iter_free(iter); - assert(i == a->n_pkgs); + if (i != a->n_pkgs) + internerr("inconsistent state in pkg array: i=%d != npkgs=%d", + i, a->n_pkgs); } /** diff --git a/lib/dpkg/pkg-db.c b/lib/dpkg/pkg-db.c index 300c1ca..7135450 100644 --- a/lib/dpkg/pkg-db.c +++ b/lib/dpkg/pkg-db.c @@ -24,7 +24,6 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> #include <stdlib.h> @@ -171,8 +170,10 @@ pkg_db_get_pkg(struct pkgset *set, const struct dpkg_arch *arch) { struct pkginfo *pkg, **pkgp; - assert(arch); - assert(arch->type != DPKG_ARCH_NONE); + if (arch == NULL) + internerr("arch argument is NULL"); + if (arch->type == DPKG_ARCH_NONE) + internerr("arch argument is none"); pkg = &set->pkg; diff --git a/lib/dpkg/pkg.c b/lib/dpkg/pkg.c index 1fc3d0c..4141fdd 100644 --- a/lib/dpkg/pkg.c +++ b/lib/dpkg/pkg.c @@ -22,9 +22,9 @@ #include <config.h> #include <compat.h> -#include <assert.h> #include <string.h> +#include <dpkg/ehandle.h> #include <dpkg/string.h> #include <dpkg/dpkg-db.h> #include <dpkg/pkg.h> @@ -42,7 +42,9 @@ pkg_set_status(struct pkginfo *pkg, enum pkgstatus status) else if (status == PKG_STAT_NOTINSTALLED) pkg->set->installed_instances--; - assert(pkg->set->installed_instances >= 0); + if (pkg->set->installed_instances < 0) + internerr("pkgset %s went into negative installed instances %d", + pkg->set->name, pkg->set->installed_instances); pkg->status = status; } diff --git a/lib/dpkg/triglib.c b/lib/dpkg/triglib.c index 57bd2a6..a067d14 100644 --- a/lib/dpkg/triglib.c +++ b/lib/dpkg/triglib.c @@ -26,7 +26,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <stdlib.h> #include <unistd.h> @@ -104,7 +103,9 @@ trig_clear_awaiters(struct pkginfo *notpend) struct trigaw *ta; struct pkginfo *aw; - assert(!notpend->trigpend_head); + if (notpend->trigpend_head) + internerr("package %s has pending triggers", + pkg_name(notpend, pnaw_always)); ta = notpend->othertrigaw_head; notpend->othertrigaw_head = NULL; @@ -400,7 +401,10 @@ trk_file_interest_change(const char *trig, struct pkginfo *pkg, fnn = trigh.namenode_find(trig, signum <= 0); if (!fnn) { - assert(signum < 0); + if (signum >= 0) + internerr("lost filename node '%s' for package %s " + "triggered to add", trig, + pkgbin_name(pkg, pkgbin, pnaw_always)); return; } @@ -627,7 +631,10 @@ trig_cicb_interest_change(const char *trig, struct pkginfo *pkg, { const struct trigkindinfo *tki = trig_classify_byname(trig); - assert(filetriggers_edited >= 0); + if (filetriggers_edited < 0) + internerr("trigger control file for package %s not read", + pkgbin_name(pkg, pkgbin, pnaw_always)); + tki->interest_change(trig, pkg, pkgbin, signum, opts); } diff --git a/m4/dpkg-compiler.m4 b/m4/dpkg-compiler.m4 index a2e68e3..a6192f6 100644 --- a/m4/dpkg-compiler.m4 +++ b/m4/dpkg-compiler.m4 @@ -56,6 +56,7 @@ AC_DEFUN([DPKG_CHECK_COMPILER_WARNINGS], [ DPKG_CHECK_COMPILER_FLAG([-Wmissing-declarations]) DPKG_CHECK_COMPILER_FLAG([-Wmissing-format-attribute]) DPKG_CHECK_COMPILER_FLAG([-Wno-missing-field-initializers]) + DPKG_CHECK_COMPILER_FLAG([-Wno-nonnull-compare]) DPKG_CHECK_COMPILER_FLAG([-Wno-tautological-constant-out-of-range-compare]) DPKG_CHECK_COMPILER_FLAG([-Wno-unused-parameter]) DPKG_CHECK_COMPILER_FLAG([-Wnull-dereference]) diff --git a/src/archives.c b/src/archives.c index b82ee4f..113b76c 100644 --- a/src/archives.c +++ b/src/archives.c @@ -29,7 +29,6 @@ #include <sys/time.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <time.h> @@ -631,7 +630,8 @@ linktosameexistingdir(const struct tar_entry *ti, const char *fname, varbuf_add_str(symlinkfn, instdir); } else { lastslash= strrchr(fname, '/'); - assert(lastslash); + if (lastslash == NULL) + internerr("tar entry filename '%s' does not contain '/'", fname); varbuf_add_buf(symlinkfn, fname, (lastslash - fname) + 1); } varbuf_add_str(symlinkfn, ti->linkname); @@ -1267,7 +1267,11 @@ void check_breaks(struct dependency *dep, struct pkginfo *pkg, char action[512]; ensure_package_clientdata(fixbydeconf); - assert(fixbydeconf->clientdata->istobe == PKG_ISTOBE_NORMAL); + + if (fixbydeconf->clientdata->istobe != PKG_ISTOBE_NORMAL) + internerr("package %s being fixed by deconf is not to be normal, " + "is to be %d", + pkg_name(pkg, pnaw_always), fixbydeconf->clientdata->istobe); sprintf(action, _("installation of %.250s"), pkgbin_name(pkg, &pkg->available, pnaw_nonambig)); @@ -1329,8 +1333,13 @@ void check_conflict(struct dependency *dep, struct pkginfo *pkg, fixbyrm->want != PKG_WANT_HOLD) || does_replace(pkg, &pkg->available, fixbyrm, &fixbyrm->installed)) && (!fixbyrm->installed.essential || fc_removeessential)))) { - assert(fixbyrm->clientdata->istobe == PKG_ISTOBE_NORMAL || - fixbyrm->clientdata->istobe == PKG_ISTOBE_DECONFIGURE); + + if (fixbyrm->clientdata->istobe != PKG_ISTOBE_NORMAL && + fixbyrm->clientdata->istobe != PKG_ISTOBE_DECONFIGURE) + internerr("package %s to be fixed by removal is not to be normal " + "nor deconfigure, is to be %d", + pkg_name(pkg, pnaw_always), fixbyrm->clientdata->istobe); + fixbyrm->clientdata->istobe = PKG_ISTOBE_REMOVE; notice(_("considering removing %s in favour of %s ..."), pkg_name(fixbyrm, pnaw_nonambig), diff --git a/src/configure.c b/src/configure.c index 8158824..3c565c6 100644 --- a/src/configure.c +++ b/src/configure.c @@ -29,7 +29,6 @@ #include <sys/stat.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <ctype.h> #include <string.h> @@ -681,7 +680,9 @@ deferred_configure(struct pkginfo *pkg) pkg_set_status(pkg, PKG_STAT_HALFCONFIGURED); } - assert(pkg->status == PKG_STAT_HALFCONFIGURED); + if (pkg->status != PKG_STAT_HALFCONFIGURED) + internerr("package %s in state %s, instead of half-configured", + pkg_name(pkg, pnaw_always), pkg_status_name(pkg)); modstatdb_note(pkg); diff --git a/src/depcon.c b/src/depcon.c index 924c53a..398d06a 100644 --- a/src/depcon.c +++ b/src/depcon.c @@ -27,7 +27,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <stdlib.h> #include <unistd.h> @@ -314,10 +313,14 @@ depisok(struct dependency *dep, struct varbuf *whynot, * Allow 250x3 for package names, versions, &c, + 250 for ourselves. */ char linebuf[1024]; - assert(dep->type == dep_depends || dep->type == dep_predepends || - dep->type == dep_breaks || dep->type == dep_conflicts || - dep->type == dep_recommends || dep->type == dep_suggests || - dep->type == dep_enhances); + if (dep->type != dep_depends && + dep->type != dep_predepends && + dep->type != dep_breaks && + dep->type != dep_conflicts && + dep->type != dep_recommends && + dep->type != dep_suggests && + dep->type != dep_enhances) + internerr("unknown dependency type %d", dep->type); if (canfixbyremove) *canfixbyremove = NULL; diff --git a/src/enquiry.c b/src/enquiry.c index b299f84..99349ba 100644 --- a/src/enquiry.c +++ b/src/enquiry.c @@ -26,7 +26,6 @@ #include <sys/types.h> -#include <assert.h> #include <string.h> #include <fcntl.h> #include <unistd.h> @@ -495,7 +494,9 @@ predeppackage(const char *const *argv) if (!dep) return 1; /* Not found. */ - assert(pkg); + if (pkg == NULL) + internerr("unexpected unfound package"); + startpkg= pkg; pkg->clientdata->istobe = PKG_ISTOBE_PREINSTALL; diff --git a/src/filesdb.c b/src/filesdb.c index 5050144..7f15772 100644 --- a/src/filesdb.c +++ b/src/filesdb.c @@ -32,7 +32,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <pwd.h> @@ -600,8 +599,13 @@ struct filenamenode *findnamenode(const char *name, enum fnnflags flags) { pointerp = bins + (str_fnv_hash(name) % (BINS)); while (*pointerp) { - /* XXX: Why is the assert needed? It's checking already added entries. */ - assert((*pointerp)->name[0] == '/'); + /* XXX: This should not be needed, but it has been a constant source + * of assertions over the years. Hopefully with the internerr() we will + * get better diagnostics. */ + if ((*pointerp)->name[0] != '/') + internerr("filename node '%s' does not start with '/'", + (*pointerp)->name); + if (strcmp((*pointerp)->name + 1, name) == 0) break; pointerp= &(*pointerp)->next; diff --git a/src/packages.c b/src/packages.c index f68c971..ae00175 100644 --- a/src/packages.c +++ b/src/packages.c @@ -27,7 +27,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <string.h> #include <fcntl.h> #include <dirent.h> @@ -232,7 +231,9 @@ void process_queue(void) { * trigger processing, w/o jumping into the next dependtry. */ dependtry++; sincenothing = 0; - assert(dependtry <= 4); + if (dependtry > 4) + internerr("exceeded dependtry %d (sincenothing=%d; queue.length=%d)", + dependtry, sincenothing, queue.length); } else if (sincenothing > queue.length * 2 + 2) { /* XXX: This probably needs moving into a new dependtry instead. */ if (progress_bytrigproc && progress_bytrigproc->trigpend_head) { @@ -242,7 +243,9 @@ void process_queue(void) { } else { dependtry++; sincenothing = 0; - assert(dependtry <= 4); + if (dependtry > 4) + internerr("exceeded dependtry %d (sincenothing=%d, queue.length=%d)", + dependtry, sincenothing, queue.length); } } @@ -250,7 +253,8 @@ void process_queue(void) { pkg_name(pkg, pnaw_always), queue.length, sincenothing, dependtry); if (pkg->status > PKG_STAT_INSTALLED) - internerr("package status (%d) > PKG_STAT_INSTALLED", pkg->status); + internerr("package %s status %d is out-of-bounds", + pkg_name(pkg, pnaw_always), pkg->status); if (setjmp(ejbuf)) { /* Give up on it from the point of view of other packages, i.e. reset @@ -295,7 +299,10 @@ void process_queue(void) { pop_error_context(ehflag_normaltidy); } - assert(!queue.length); + + if (queue.length) + internerr("finished package processing with non-empty queue length %d", + queue.length); } /*** Dependency processing - common to --configure and --remove. ***/ @@ -424,7 +431,11 @@ deppossi_ok_found(struct pkginfo *possdependee, struct pkginfo *requiredby, return FOUND_OK; } if (possdependee->status == PKG_STAT_TRIGGERSAWAITED) { - assert(possdependee->trigaw.head); + if (possdependee->trigaw.head == NULL) + internerr("package %s in state %s, has no awaited triggers", + pkg_name(possdependee, pnaw_always), + pkg_status_name(possdependee)); + if (removing || !(f_triggers || possdependee->clientdata->istobe == PKG_ISTOBE_INSTALLNEW)) { diff --git a/src/script.c b/src/script.c index ac58779..ecf4a94 100644 --- a/src/script.c +++ b/src/script.c @@ -25,7 +25,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <unistd.h> @@ -137,7 +136,10 @@ maintscript_pre_exec(struct command *cmd) if (instdirlen == 0 || fc_script_chrootless) return cmd->filename; - assert(strlen(cmd->filename) >= instdirlen); + if (strlen(cmd->filename) < instdirlen) + internerr("maintscript name '%s' length < instdir length %zd", + cmd->filename, instdirlen); + return cmd->filename + instdirlen; } diff --git a/src/trigproc.c b/src/trigproc.c index b9681a2..9c73d4c 100644 --- a/src/trigproc.c +++ b/src/trigproc.c @@ -25,7 +25,6 @@ #include <sys/stat.h> -#include <assert.h> #include <fcntl.h> #include <stdlib.h> @@ -335,8 +334,11 @@ check_trigger_cycle(struct pkginfo *processing_now) debug(dbg_triggers, "check_triggers_cycle pnow=%s giveup=%s", pkg_name(processing_now, pnaw_always), pkg_name(giveup, pnaw_always)); - assert(giveup->status == PKG_STAT_TRIGGERSAWAITED || - giveup->status == PKG_STAT_TRIGGERSPENDING); + if (giveup->status != PKG_STAT_TRIGGERSAWAITED && + giveup->status != PKG_STAT_TRIGGERSPENDING) + internerr("package %s in non-trigger state %s", + pkg_name(giveup, pnaw_always), + pkg_status_name(giveup)); pkg_set_status(giveup, PKG_STAT_HALFCONFIGURED); modstatdb_note(giveup); print_error_perpackage(_("triggers looping, abandoned"), @@ -367,8 +369,11 @@ trigproc(struct pkginfo *pkg, enum trigproc_type type) if (pkg->trigpend_head) { enum dep_check ok; - assert(pkg->status == PKG_STAT_TRIGGERSPENDING || - pkg->status == PKG_STAT_TRIGGERSAWAITED); + if (pkg->status != PKG_STAT_TRIGGERSPENDING && + pkg->status != PKG_STAT_TRIGGERSAWAITED) + internerr("package %s in non-trigger state %s", + pkg_name(pkg, pnaw_always), + pkg_status_name(pkg)); if (dependtry > 1) { gaveup = check_trigger_cycle(pkg); diff --git a/src/unpack.c b/src/unpack.c index ed24272..f43c01e 100644 --- a/src/unpack.c +++ b/src/unpack.c @@ -28,7 +28,6 @@ #include <sys/stat.h> #include <sys/wait.h> -#include <assert.h> #include <errno.h> #include <string.h> #include <time.h> @@ -859,8 +858,12 @@ pkg_disappear_others(struct pkginfo *pkg) debug(dbg_veryverbose, "process_archive checking disappearance %s", pkg_name(otherpkg, pnaw_always)); - assert(otherpkg->clientdata->istobe == PKG_ISTOBE_NORMAL || - otherpkg->clientdata->istobe == PKG_ISTOBE_DECONFIGURE); + + if (otherpkg->clientdata->istobe != PKG_ISTOBE_NORMAL && + otherpkg->clientdata->istobe != PKG_ISTOBE_DECONFIGURE) + internerr("disappearing package %s is not to be normal or deconfigure, " + "is to be %d", + pkg_name(otherpkg, pnaw_always), otherpkg->clientdata->istobe); for (cfile = otherpkg->clientdata->files; cfile && strcmp(cfile->namenode->name, "/.") == 0; @@ -1226,7 +1229,10 @@ void process_archive(const char *filename) { oldversionstatus= pkg->status; - assert(oldversionstatus <= PKG_STAT_INSTALLED); + if (oldversionstatus > PKG_STAT_INSTALLED) + internerr("package %s state %d is out-of-bounds", + pkg_name(pkg, pnaw_always), oldversionstatus); + debug(dbg_general,"process_archive oldversionstatus=%s", statusstrings[oldversionstatus]); @@ -1516,7 +1522,9 @@ void process_archive(const char *filename) { if (otherpkg->installed.arch != pkg->installed.arch) continue; - assert(otherpkg->status == PKG_STAT_NOTINSTALLED); + if (otherpkg->status != PKG_STAT_NOTINSTALLED) + internerr("other package %s instance in state %s instead of not-installed", + pkg_name(otherpkg, pnaw_always), pkg_status_name(otherpkg)); pkg_blank(otherpkg); } diff --git a/utils/start-stop-daemon.c b/utils/start-stop-daemon.c index 611340e..faea3d1 100644 --- a/utils/start-stop-daemon.c +++ b/utils/start-stop-daemon.c @@ -80,7 +80,6 @@ #include <sys/select.h> #include <sys/ioctl.h> -#include <assert.h> #include <errno.h> #include <limits.h> #include <time.h> @@ -959,7 +958,9 @@ parse_schedule(const char *schedule_str) schedule[count].value = repeatat; count++; } - assert(count == schedule_length); + if (count != schedule_length) + BUG("count=%d != schedule_length=%d", + count, schedule_length); } } @@ -2520,7 +2521,8 @@ run_stop_schedule(void) else continue; default: - assert(!"schedule[].type value must be valid"); + BUG("schedule[%d].type value %d is not valid", + position, schedule[position].type); } } -- Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/dpkg/dpkg.git
