You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by ia...@apache.org on 2001/08/23 00:40:13 UTC
cvs commit: apr-util/test testdbm.c
ianh 01/08/22 15:40:12
Modified: test testdbm.c
Log:
added extra checking of return codes
added new command 'names' which returns the name of the db.
for multi-line submissions, a 0 length line signals you want to stop
changed malloc to apr_palloc
added a atexit to make sure all cleans are called
Revision Changes Path
1.9 +71 -32 apr-util/test/testdbm.c
Index: testdbm.c
===================================================================
RCS file: /home/cvs/apr-util/test/testdbm.c,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- testdbm.c 2001/08/22 15:40:29 1.8
+++ testdbm.c 2001/08/22 22:40:12 1.9
@@ -81,12 +81,13 @@
#define DERROR 0
#define DLOOK 1
-#define DINSERT 2
+
#define DDELETE 3
#define DCAT 4
#define DBUILD 5
#define DPRESS 6
#define DCREAT 7
+#define DNAME 8
#define LINEMAX 8192
@@ -101,22 +102,23 @@
{ "fetch", DLOOK, APR_DBM_READONLY, },
{ "get", DLOOK, APR_DBM_READONLY, },
{ "look", DLOOK, APR_DBM_READONLY, },
- { "add", DINSERT, APR_DBM_READWRITE, },
- { "insert", DINSERT, APR_DBM_READWRITE, },
- { "store", DINSERT, APR_DBM_READWRITE, },
+ { "add", DBUILD, APR_DBM_READWRITE, },
+ { "insert", DBUILD, APR_DBM_READWRITE, },
+ { "store", DBUILD, APR_DBM_READWRITE, },
{ "delete", DDELETE, APR_DBM_READWRITE, },
{ "remove", DDELETE, APR_DBM_READWRITE, },
{ "dump", DCAT, APR_DBM_READONLY, },
{ "list", DCAT, APR_DBM_READONLY, },
{ "cat", DCAT, APR_DBM_READONLY, },
+ { "build", DBUILD, APR_DBM_RWCREATE, }, /** this one creates the DB */
+ { "creat", DCREAT, APR_DBM_RWCREATE, },
+ { "new", DCREAT, APR_DBM_RWCREATE, },
+ { "names", DNAME, APR_DBM_READONLY, },
#if 0
- { "creat", DCREAT, APR_DBM_RWCREATE | O_TRUNC, },
- { "new", DCREAT, APR_DBM_RWCREATE | O_TRUNC, },
+ {"squash", DPRESS, APR_DBM_READWRITE,},
+ {"compact", DPRESS, APR_DBM_READWRITE,},
+ {"compress", DPRESS, APR_DBM_READWRITE,},
#endif
- { "build", DBUILD, APR_DBM_RWCREATE, },
- { "squash", DPRESS, APR_DBM_READWRITE, },
- { "compact", DPRESS, APR_DBM_READWRITE, },
- { "compress", DPRESS, APR_DBM_READWRITE, },
};
#define CTABSIZ (sizeof (cmds)/sizeof (cmd))
@@ -125,7 +127,8 @@
static void badk(const char *word);
static const cmd *parse(const char *str);
static void prdatum(FILE *stream, apr_datum_t d);
-static void oops(const char *s1, const char *s2);
+static void oops(apr_dbm_t *dbm, apr_status_t rv, const char *s1,
+ const char *s2);
int main(int argc, const char * const * argv)
@@ -138,6 +141,7 @@
(void) apr_initialize();
apr_pool_create(&pool, NULL);
+ atexit(apr_terminate);
(void) apr_getopt_init(&os, pool, argc, argv);
@@ -145,17 +149,17 @@
while (apr_getopt(os, "R", &optch, &optarg) == APR_SUCCESS)
switch (optch) {
- case 'R': /* raw processing */
+ case 'R': /* raw processing */
rflag++;
break;
default:
- oops("(unknown option) usage: %s", usage);
+ oops(NULL, APR_EGENERAL, "(unknown option) usage: %s", usage);
break;
}
if (os->ind + 2 > argc)
- oops("usage: %s", usage);
+ oops(NULL, APR_EGENERAL, "usage: %s", usage);
if ((act = parse(argv[os->ind])) == NULL)
badk(argv[os->ind]);
@@ -163,7 +167,7 @@
doit(act, argv[os->ind], pool);
apr_pool_destroy(pool);
- apr_terminate();
+
return 0;
}
@@ -176,17 +180,21 @@
char *op;
int n;
char *line;
+ const char *use1;
+ const char *use2;
#ifdef TIME
long start;
extern long time();
#endif
- if (apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool)
- != APR_SUCCESS)
- oops("cannot open: %s", file);
- if ((line = (char *) malloc(LINEMAX)) == NULL)
- oops("%s: cannot get memory", "line alloc");
+ rv = apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool);
+ if (rv != APR_SUCCESS)
+ oops(db, rv, "cannot open: %s", file);
+
+ if ((line = (char *) apr_palloc(pool,LINEMAX)) == NULL) {
+ oops(NULL, APR_EGENERAL, "%s: cannot get memory", "line alloc");
+ }
switch (act->scode) {
@@ -194,6 +202,9 @@
while (fgets(line, LINEMAX, stdin) != NULL) {
n = strlen(line) - 1;
line[n] = 0;
+ if (n == 0)
+ break;
+
key.dptr = line;
key.dsize = n;
rv = apr_dbm_fetch(db, key, &val);
@@ -205,13 +216,15 @@
prdatum(stderr, key);
fprintf(stderr, ": not found.\n");
}
- break;
- case DINSERT:
break;
+
case DDELETE:
while (fgets(line, LINEMAX, stdin) != NULL) {
n = strlen(line) - 1;
line[n] = 0;
+ if (n == 0)
+ break;
+
key.dptr = line;
key.dsize = n;
if (apr_dbm_delete(db, key) != APR_SUCCESS) {
@@ -221,15 +234,21 @@
}
break;
case DCAT:
- if (apr_dbm_firstkey(db, &key) != APR_SUCCESS)
- oops("could not fetch first key: %s", file);
+ rv = apr_dbm_firstkey(db, &key);
+ if (rv != APR_SUCCESS)
+ oops(db, rv, "could not fetch first key: %s", file);
- for (; key.dptr != 0; (void) apr_dbm_nextkey(db, &key)) {
+ while (key.dptr != NULL) {
prdatum(stdout, key);
putchar('\t');
- (void) apr_dbm_fetch(db, key, &val);
+ rv = apr_dbm_fetch(db, key, &val);
+ if (rv != APR_SUCCESS)
+ oops(db, rv, "apr_dbm_fetch", "failure");
prdatum(stdout, val);
putchar('\n');
+ rv = apr_dbm_nextkey(db, &key);
+ if (rv != APR_SUCCESS)
+ oops(db, rv, "NextKey", "failure");
}
break;
case DBUILD:
@@ -239,6 +258,9 @@
while (fgets(line, LINEMAX, stdin) != NULL) {
n = strlen(line) - 1;
line[n] = 0;
+ if (n == 0)
+ break;
+
key.dptr = line;
if ((op = strchr(line, '\t')) != 0) {
key.dsize = op - line;
@@ -247,12 +269,13 @@
val.dsize = line + n - op;
}
else
- oops("bad input: %s", line);
-
- if (apr_dbm_store(db, key, val) != APR_SUCCESS) {
+ oops(NULL, APR_EGENERAL, "bad input: %s", line);
+
+ rv = apr_dbm_store(db, key, val);
+ if (rv != APR_SUCCESS) {
prdatum(stderr, key);
fprintf(stderr, ": ");
- oops("store: %s", "failed");
+ oops(db, rv, "store: %s", "failed");
}
}
#ifdef TIME
@@ -263,6 +286,10 @@
break;
case DCREAT:
break;
+ case DNAME:
+ apr_dbm_get_usednames(pool, file, &use1, &use2);
+ fprintf(stderr, "%s %s\n", use1, use2);
+ break;
}
apr_dbm_close(db);
@@ -287,7 +314,7 @@
{
int i = CTABSIZ;
const cmd *p;
-
+
for (p = cmds; i--; p++)
if (strcmp(p->sname, str) == 0)
return p;
@@ -313,13 +340,25 @@
}
}
-static void oops(const char *s1, const char *s2)
+static void oops(apr_dbm_t * dbm, apr_status_t rv, const char *s1,
+ const char *s2)
{
+ char errbuf[200];
+
if (progname)
fprintf(stderr, "%s: ", progname);
fprintf(stderr, s1, s2);
if (errno > 0 && errno < sys_nerr)
fprintf(stderr, " (%s)", sys_errlist[errno]);
fprintf(stderr, "\n");
+ if (rv != APR_SUCCESS) {
+ apr_strerror(rv, errbuf, sizeof(errbuf));
+ fprintf(stderr, "APR Error %d - %s\n", rv, errbuf);
+
+ if (dbm) {
+ apr_dbm_geterror(dbm, &rv, errbuf, sizeof(errbuf));
+ fprintf(stderr, "APR_DB Error %d - %s\n", rv, errbuf);
+ }
+ }
exit(1);
}
Possible segfault in apr_pvsprintf
Posted by Sander Striker <st...@apache.org>.
Hi,
In addition to the possible segfault in apr_palloc
when no abort function is set and we're out of mem,
there is another possible segfault in apr_pvsprintf.
This one is even worse, because there is no abort
function.
Excerpt from apr_pools.c:psprintf_flush():
/* must try another blok */
#if APR_HAS_THREADS
apr_lock_acquire(alloc_mutex);
#endif
nblok = new_block(2 * cur_len, NULL);
#if APR_HAS_THREADS
apr_lock_release(alloc_mutex);
#endif
memcpy(nblok->h.first_avail, blok->h.first_avail, cur_len);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ps->vbuff.curpos = nblok->h.first_avail + cur_len;
/* save a byte for the NUL terminator */
ps->vbuff.endpos = nblok->h.endp - 1;
The code will segfault when out of mem occurs within psprintf_flush
at the moment at the marked line.
Sander
RE: Pools behaviour, WAS: RE: cvs commit: apr-util/test testdbm.c
Posted by Sander Striker <st...@apache.org>.
> On Thu, Aug 23, 2001 at 11:04:45AM +0200, Sander Striker wrote:
> > ...
> > Greg> apr_palloc() will never return NULL. No need for such a
> complex test.
> >
> > It won't? You mean that in httpd there is always an abortfunc present?
>
> There should be an abortfunc, yes. ISTR once looking and finding it wasn't
> set (but it should).
>
> APR and httpd and every other APR user that I'm aware of assume that
> apr_palloc and friends succeed or abort. Everything is designed and built
> that way.
>
> Thus: until we decide to change that assumption, it is useless clutter to
> check for NULL results.
Agreed, that was not the issue I was pointing out. httpd isn't doing NULL
checking for apr_palloc anywhere, that should be maintained. However,
this implies that an abort function should be set somewhere.
For applications not setting an abort function there should at least be
the oppurtunity to check for NULL results of apr_palloc.
Applications that set an abort functions can safely assume that apr_palloc
returns something valid or calls the abort function.
AFAIK httpd-2.0 _doesn't_ set an abort function. I believe Ryan said
something on this a few months back.
> > Oh, I see, you are saying that _without_ an abort
> > function it will segfault due to the marked line?
>
> I had no idea about that segfault. It is just a typical example of what I
> described above.
Not really. Pools don't enforce you to set an abort function. If it isn't
set the pools code shouldn't segfault.
> We can talk on and on about whether that is "appropriate" or not, but it
> isn't going to change in any near or mid term that I could imagine.
Well, the reason I am interested is because I am working on yet another
attempt to optimize the pools code. Justin ran some 'benchmarks' last
night,
but we need to run them again to be sure there is a performance increase.
If so I'll be posting a patch for review (it also removes the locking issues
we have regarding to the global free list).
In that new code I would like to do what is appropiate and return NULL
when we have no mem and no abort function.
Sander
Re: Pools behaviour, WAS: RE: cvs commit: apr-util/test testdbm.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Aug 23, 2001 at 11:04:45AM +0200, Sander Striker wrote:
> ...
> Greg> apr_palloc() will never return NULL. No need for such a complex test.
>
> It won't? You mean that in httpd there is always an abortfunc present?
There should be an abortfunc, yes. ISTR once looking and finding it wasn't
set (but it should).
APR and httpd and every other APR user that I'm aware of assume that
apr_palloc and friends succeed or abort. Everything is designed and built
that way.
Thus: until we decide to change that assumption, it is useless clutter to
check for NULL results.
> Oh, I see, you are saying that _without_ an abort
> function it will segfault due to the marked line?
I had no idea about that segfault. It is just a typical example of what I
described above.
We can talk on and on about whether that is "appropriate" or not, but it
isn't going to change in any near or mid term that I could imagine.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Pools behaviour, WAS: RE: cvs commit: apr-util/test testdbm.c
Posted by Sander Striker <st...@apache.org>.
...
Ian>> @@ -176,17 +180,21 @@
Ian>> char *op;
Ian>> int n;
Ian>> char *line;
Ian>> + const char *use1;
Ian>> + const char *use2;
Ian>> #ifdef TIME
Ian>> long start;
Ian>> extern long time();
Ian>> #endif
Ian>>
Ian>> - if (apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool)
Ian>> - != APR_SUCCESS)
Ian>> - oops("cannot open: %s", file);
Ian>>
Ian>> - if ((line = (char *) malloc(LINEMAX)) == NULL)
Ian>> - oops("%s: cannot get memory", "line alloc");
Ian>> + rv = apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool);
Ian>> + if (rv != APR_SUCCESS)
Ian>> + oops(db, rv, "cannot open: %s", file);
Ian>> +
Ian>> + if ((line = (char *) apr_palloc(pool,LINEMAX)) == NULL) {
Ian>> + oops(NULL, APR_EGENERAL, "%s: cannot get memory", "line
alloc");
Ian>> + }
Greg> apr_palloc() will never return NULL. No need for such a complex test.
It won't? You mean that in httpd there is always an abortfunc present?
>From apr_pools.c:malloc_block():
blok = (union block_hdr *) DO_MALLOC(size + sizeof(union block_hdr));
if (blok == NULL) {
/* ### keep this fprintf here? */
fprintf(stderr, "Ouch! malloc failed in malloc_block()\n");
if (abortfunc != NULL) {
(void) (*abortfunc)(APR_ENOMEM);
}
return NULL;
}
And from apr_pools.c:apr_palloc():
blok = new_block(size, a->apr_abort);
a->last->h.next = blok;
a->last = blok;
#ifdef APR_POOL_DEBUG
blok->h.owning_pool = a;
#endif
#if APR_HAS_THREADS
if (alloc_mutex) {
apr_lock_release(alloc_mutex);
}
#endif
first_avail = blok->h.first_avail;
blok->h.first_avail += size;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
return (void *) first_avail;
#endif
}
Oh, I see, you are saying that _without_ an abort
function it will segfault due to the marked line?
Is this reasonable behaviour? I mean a simple
change to:
if ((blok = new_block(size, a->apr_abort)) == NULL)
return NULL;
might be more intuitive than having it segfault in
apr_palloc IMHO.
Sander
Re: cvs commit: apr-util/test testdbm.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Aug 22, 2001 at 10:40:13PM -0000, ianh@apache.org wrote:
>...
> --- testdbm.c 2001/08/22 15:40:29 1.8
> +++ testdbm.c 2001/08/22 22:40:12 1.9
As a general note: testdbm.c is derived from dbu.c in the (very old) SDBM
tarball. As such, it is written using a style/technique/whatever circa 1991.
Please feel free to revamp at will to make it feel like a "real" tool.
>...
> @@ -176,17 +180,21 @@
> char *op;
> int n;
> char *line;
> + const char *use1;
> + const char *use2;
> #ifdef TIME
> long start;
> extern long time();
> #endif
>
> - if (apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool)
> - != APR_SUCCESS)
> - oops("cannot open: %s", file);
>
> - if ((line = (char *) malloc(LINEMAX)) == NULL)
> - oops("%s: cannot get memory", "line alloc");
> + rv = apr_dbm_open(&db, file, act->flags, APR_OS_DEFAULT, pool);
> + if (rv != APR_SUCCESS)
> + oops(db, rv, "cannot open: %s", file);
> +
> + if ((line = (char *) apr_palloc(pool,LINEMAX)) == NULL) {
> + oops(NULL, APR_EGENERAL, "%s: cannot get memory", "line alloc");
> + }
apr_palloc() will never return NULL. No need for such a complex test.
>...
> @@ -263,6 +286,10 @@
> break;
> case DCREAT:
> break;
> + case DNAME:
> + apr_dbm_get_usednames(pool, file, &use1, &use2);
> + fprintf(stderr, "%s %s\n", use1, use2);
> + break;
There are tabs in that source code (Apache code should not have tabs).
Please fix your editor.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/