From dean-clamav@arctic.org Sat Mar 6 19:21:05 2004 Date: Sat, 6 Mar 2004 19:19:51 -0800 (PST) From: dean gaudet To: bugs@clamav.net Subject: 3.3x speed improvement X-comment: visit http://arctic.org/~dean/legal for information regarding copyright and disclaimer. while investigating whether i wanted to enable clamav on all my email i discovered that it was using /dev/urandom... "using" is a light term -- it is literally abusing /dev/urandom. cl_rndnum uses buffered FILE i/o, so even though it reads only a few bytes the underlying library has read and wasted typically 4096 bytes by filling its buffer. cl_gentemp calls cl_rndnum 32 times! as a result this consumes 128KiB of /dev/urandom. /dev/urandom is cryptographically secure which means it is typically the output of something such as SHA1 -- generating and throwing away 128KiB of SHA1 is a terrible waste of cpu. i've made several changes: - replace the cl_gentemp function with three functions cl_mkstemp, cl_mkftemp, and cl_mkdtemp ... these are based on the c library functions mkstemp() and mkdtemp. while these functions exist in at least *bsd and linux, they may not exist everywhere. however these are the defacto safe & secure temp file creation library functions -- and there are implementations available in other packages for portable purposes, i haven't bothered to include a portable implementation. - eliminate the use of cl_rndnum() in the stream support -- i don't see why this is there... if you ask to bind to port 0 the kernel will select a port. so i've replaced the code with a bind to port 0. i didn't test this portion fo the change at all. - delete cl_rndnum, cl_gentemp, and the urandom configure stuff my basis for claiming a 3.3x speed improvement is a mailbox i have with 370 messages in it (all of which matched the windows exe signature in base64). i used formail to break this mailbox into 370 files, and then ran: clamscan --mbox * before: 5.600u 14.480s 0:22.51 89.2% 0+0k 0+0io 229pf+0w after: 5.730u 0.520s 0:06.81 91.7% 0+0k 0+0io 1194pf+0w this is essentially what you'd expect -- all that /dev/urandom time is kernel time. notice it's gone. i recognize this change affects exported library functions ... but i hope you do consider it... the code as it exists today is only probabilistically race free -- this change to use mkstemp()/mkdtemp() is guaranteed race free. i think the perf benefit speaks for itself. thanks -dean diff -ru clamav-0.67.old/debian/changelog clamav-0.67/debian/changelog --- clamav-0.67.old/debian/changelog 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/debian/changelog 2004-03-06 19:07:42.000000000 -0800 @@ -1,3 +1,19 @@ +clamav (0.67-6.dg1) unstable; urgency=low + + * the code was abusing /dev/urandom in a manner which would completely + destroy any entropy in the system... is a total waste of computation + time since /dev/urandom generates cryptographic random numbers + typically through the use of SHA1. what's worse is the cl_gentemp + function used cl_rndnum in a manner that consumed 128KiB of + /dev/urandom!! here's how it was fixed: + - replace the cl_rndnum(60000) code in the "STREAM" support with + code that lets the kernel decide a port number efficiently + - replace cl_gentemp with cl_mkstemp, cl_mkftemp, cl_mkdtemp, + modelled off standard mkstemp(3) and mkdtemp(3) c lib functions + - delete cl_rndnum + + -- dean gaudet Sat, 6 Mar 2004 16:09:32 -0800 + clamav (0.67-6) unstable; urgency=low * Add line break in clamav-milter.8 (closes: #235452) * Add a longer diff -ru clamav-0.67.old/clamav-config.h.in clamav-0.67/clamav-config.h.in --- clamav-0.67.old/clamav-config.h.in 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/clamav-config.h.in 2004-03-06 18:01:19.000000000 -0800 @@ -54,9 +54,6 @@ /* os is solaris */ #undef C_SOLARIS -/* use /dev/urandom */ -#undef C_URANDOM - /* Path to virus database directory. */ #undef DATADIR diff -ru clamav-0.67.old/clamd/scanner.c clamav-0.67/clamd/scanner.c --- clamav-0.67.old/clamd/scanner.c 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/clamd/scanner.c 2004-03-06 16:12:06.000000000 -0800 @@ -186,43 +186,52 @@ int scanstream(int odesc, unsigned long int *scanned, const struct cl_node *root, const struct cl_limits *limits, int options, const struct cfgstruct *copt) { - int ret, portscan = CL_DEFAULT_MAXPORTSCAN, sockfd, port, acceptd, tmpd, bread, count; + int ret, portscan = CL_DEFAULT_MAXPORTSCAN, sockfd, acceptd, tmpd, bread, count; long int size = 0, maxsize = 0; - short bound = 0; char *virname, buff[32768]; struct sockaddr_in server; struct cfgstruct *cpt; FILE *tmp = NULL; struct pollfd poll_data[1]; - while(!bound && portscan--) { - if((port = cl_rndnum(60000)) < 1024) - port += 2139; - - memset((char *) &server, 0, sizeof(server)); - server.sin_family = AF_INET; - server.sin_port = htons(port); - server.sin_addr.s_addr = INADDR_ANY; - - if((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1) - continue; + sockfd = socket(AF_INET, SOCK_STREAM, 0); + if(sockfd == -1) { + int save_errno = errno; + mdprintf(odesc, "ERROR\n"); + logg("!ScanStream: can't open socket: %s\n", strerror(save_errno)); + return -1; + } - if(bind(sockfd, (struct sockaddr *) &server, sizeof(struct sockaddr_in)) == -1) - close(sockfd); - else - bound = 1; + memset((char *) &server, 0, sizeof(server)); + server.sin_family = AF_INET; + server.sin_port = 0; // let kernel choose port + server.sin_addr.s_addr = INADDR_ANY; + if(bind(sockfd, (struct sockaddr *) &server, sizeof(struct sockaddr_in)) == -1) { + int save_errno = errno; + close(sockfd); + mdprintf(odesc, "ERROR\n"); + logg("!ScanStream: can't bind socket: %s\n", strerror(save_errno)); + return -1; + } + if(getsockname(sockfd, (struct sockaddr *)&server, &ret) == -1) { + int save_errno = errno; + close(sockfd); + mdprintf(odesc, "ERROR\n"); + logg("!ScanStream: can't getsockname: %s\n", strerror(save_errno)); + return -1; } - if(!bound && !portscan) { + if(listen(sockfd, 1) == -1) { + int save_errno = errno; + close(sockfd); mdprintf(odesc, "ERROR\n"); - logg("!ScanStream: Can't find any free port.\n"); + logg("!ScanStream: can't listen: %s\n", strerror(save_errno)); return -1; - } else { - listen(sockfd, 1); - mdprintf(odesc, "PORT %d\n", port); } + mdprintf(odesc, "PORT %d\n", ntohs(server.sin_port)); + poll_data[0].fd = sockfd; poll_data[0].events = POLLIN; poll_data[0].revents = 0; @@ -250,7 +259,7 @@ } - logg("*Accepted connection on port %d, fd %d\n", port, acceptd); + logg("*Accepted connection on port %d, fd %d\n", ntohs(server.sin_port), acceptd); poll_data[0].fd = acceptd; poll_data[0].events = POLLIN; diff -ru clamav-0.67.old/clamscan/manager.c clamav-0.67/clamscan/manager.c --- clamav-0.67.old/clamscan/manager.c 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/clamscan/manager.c 2004-03-06 17:11:12.000000000 -0800 @@ -202,9 +202,9 @@ } /* generate the temporary directory */ - dir = cl_gentemp(tmpdir); - if(mkdir(dir, 0700)) { - mprintf("@Can't create the temporary directory %s\n", dir); + dir = cl_mkdtemp(tmpdir); + if(dir == NULL) { + mprintf("@Can't create the temporary directory in %s: %s\n", tmpdir, strerror(errno)); exit(63); /* critical */ } @@ -496,9 +496,9 @@ /* generate the temporary directory */ - gendir = cl_gentemp(tmpdir); - if(mkdir(gendir, 0700)) { - mprintf("@Can't create the temporary directory %s\n", gendir); + gendir = cl_mkdtemp(tmpdir); + if(gendir == NULL) { + mprintf("@Can't create the temporary directory %s: %s\n", tmpdir, strerror(errno)); exit(63); /* critical */ } @@ -697,9 +697,9 @@ } /* generate the temporary directory */ - gendir = cl_gentemp(tmpdir); - if(mkdir(gendir, 0700)) { - mprintf("@Can't create the temporary directory %s\n", gendir); + gendir = cl_mkdtemp(tmpdir); + if(gendir == NULL) { + mprintf("@Can't create the temporary directory in %s: %s\n", tmpdir, strerror(errno)); exit(63); /* critical */ } diff -ru clamav-0.67.old/configure.in clamav-0.67/configure.in --- clamav-0.67.old/configure.in 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/configure.in 2004-03-06 17:59:58.000000000 -0800 @@ -107,11 +107,6 @@ [ --disable-cr Don't link with C reentrant library (BSD) ], disable_cr=yes,) -test_urandom=yes -AC_ARG_ENABLE(urandom, -[ --disable-urandom Disable test for /dev/urandom], -test_urandom=no,) - AC_ARG_ENABLE(id-check, [ --enable-id-check Use id utility instead of /etc/passwd parsing], use_id="yes", use_id="no") @@ -203,16 +198,6 @@ AM_CONDITIONAL(INSTALL_CLAMAV_CONF, test ! -r "$cfg_dir/clamav.conf") AM_CONDITIONAL(INSTALL_FRESHCLAM_CONF, test ! -r "$cfg_dir/freshclam.conf") -if test "$test_urandom" = "yes" -then - if test -r /dev/urandom ; then - AC_MSG_RESULT(/dev/(u)random detected.) - AC_DEFINE(C_URANDOM,1,[use /dev/urandom]) - else - AC_MSG_RESULT(/dev/(u)random not detected - using weak software rand()) - fi -fi - # tcpwrappers support # rules from http://ma.ph-freiburg.de/tng/tng-technical/2002-01/msg00094.html AC_ARG_WITH(tcpwrappers, diff -ru clamav-0.67.old/freshclam/manager.c clamav-0.67/freshclam/manager.c --- clamav-0.67.old/freshclam/manager.c 2004-02-15 05:20:29.000000000 -0800 +++ clamav-0.67/freshclam/manager.c 2004-03-06 18:09:48.000000000 -0800 @@ -37,6 +37,7 @@ #include #include #include +#include #include #include "others.h" @@ -120,6 +121,7 @@ char *tempname, ipaddr[16]; const char *proxy = NULL, *user = NULL, *pass = NULL; int flevel = cl_retflevel(); + int tempfd; if((current = cl_cvdhead(localname)) == NULL) nodb = 1; @@ -211,12 +213,14 @@ }; /* end */ - /* temporary file is created in clamav's directory thus we don't need - * to create it immediately because race condition is not possible here - */ - tempname = cl_gentemp("."); + tempfd = cl_mkstemp(".", &tempname); + if (tempfd == -1) { + mprintf("@Can't create temporary file: %s", strerror(errno)); + close(hostfd); + return 52; + } - if(get_database(remotename, hostfd, tempname, hostname, proxy, user, pass)) { + if(get_database(remotename, hostfd, tempfd, hostname, proxy, user, pass)) { mprintf("@Can't download %s from %s\n", remotename, ipaddr); unlink(tempname); free(tempname); @@ -443,10 +447,10 @@ /* njh@bandsman.co.uk: added proxy support */ /* TODO: use a HEAD instruction to see if the file has been changed */ -int get_database(const char *dbfile, int socketfd, const char *file, const char *hostname, const char *proxy, const char *user, const char *pass) +int get_database(const char *dbfile, int socketfd, int fd, const char *hostname, const char *proxy, const char *user, const char *pass) { char cmd[512], buffer[FILEBUFF], *ch; - int bread, fd, i, rot = 0; + int bread, i, rot = 0; char *remotename = NULL, *authorization = NULL; const char *rotation = "|/-\\"; @@ -469,16 +473,6 @@ } } -#ifdef C_CYGWIN - if((fd = open(file, O_WRONLY|O_CREAT|O_EXCL|O_BINARY, 0644)) == -1) { -#else - if((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0644)) == -1) { -#endif - mprintf("@Can't open new file %s to write\n", file); - perror("open"); - return -1; - } - #ifdef NO_SNPRINTF sprintf(cmd, "GET %s/%s HTTP/1.1\r\n" "Host: %s\r\n%s" diff -ru clamav-0.67.old/freshclam/manager.h clamav-0.67/freshclam/manager.h --- clamav-0.67.old/freshclam/manager.h 2004-02-15 03:44:11.000000000 -0800 +++ clamav-0.67/freshclam/manager.h 2004-03-06 17:48:13.000000000 -0800 @@ -30,7 +30,7 @@ struct cl_cvd *remote_cvdhead(const char *file, int socketfd, const char *hostname, const char *proxy, const char *user, const char *pass); -int get_database(const char *dbfile, int socketfd, const char *file, const char *hostname, const char *proxy, const char *user, const char *pass); +int get_database(const char *dbfile, int socketfd, int filefd, const char *hostname, const char *proxy, const char *user, const char *pass); unsigned int fmt_base64(char* dest,const char* src,unsigned int len); diff -ru clamav-0.67.old/libclamav/clamav.h clamav-0.67/libclamav/clamav.h --- clamav-0.67.old/libclamav/clamav.h 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/libclamav/clamav.h 2004-03-06 18:09:04.000000000 -0800 @@ -22,6 +22,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" @@ -159,11 +160,14 @@ /* encode a buffer 'string' length of 'len' to a hexadecimal string */ extern char *cl_str2hex(const char *string, unsigned int len); -/* generate a pseudo-random number */ -extern unsigned int cl_rndnum(unsigned int max); +/* generate a unique subdirectory of temporary directory */ +char *cl_mkdtemp(const char *dir); -/* generate unique file name in temporary directory */ -char *cl_gentemp(const char *dir); +/* generate a unique file name, return -1 on error, 0 on success */ +int cl_mkftemp(const char *dir, char **fname, FILE **fp); + +/* generate a unique file name, return -1 on error, open fd on success */ +int cl_mkstemp(const char *dir, char **fname); #ifdef __cplusplus }; diff -ru clamav-0.67.old/libclamav/cvd.c clamav-0.67/libclamav/cvd.c --- clamav-0.67.old/libclamav/cvd.c 2004-02-15 03:44:12.000000000 -0800 +++ clamav-0.67/libclamav/cvd.c 2004-03-06 18:04:58.000000000 -0800 @@ -29,6 +29,7 @@ #include #include #include +#include #include "clamav.h" #include "others.h" @@ -344,9 +345,9 @@ tmpdir = "/tmp"; #endif - dir = cl_gentemp(tmpdir); - if(mkdir(dir, 0700)) { - cli_errmsg("cli_cvdload(): Can't create temporary directory %s\n", dir); + dir = cl_mkdtemp(tmpdir); + if(dir == NULL) { + cli_errmsg("cli_cvdload(): Can't create temporary directory in %s: %s\n", tmpdir, strerror(errno)); return CL_ETMPDIR; } @@ -363,11 +364,9 @@ /* start */ - tmp = cl_gentemp(tmpdir); - if((tmpd = fopen(tmp, "wb+")) == NULL) { - cli_errmsg("Can't create temporary file %s\n", tmp); + if(cl_mkftemp(tmpdir, &tmp, &tmpd) == -1) { + cli_errmsg("Can't create temporary file in %s: %s\n", tmpdir, strerror(errno)); free(dir); - free(tmp); return CL_ETMPFILE; } diff -ru clamav-0.67.old/libclamav/others.c clamav-0.67/libclamav/others.c --- clamav-0.67.old/libclamav/others.c 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/libclamav/others.c 2004-03-06 18:29:49.000000000 -0800 @@ -245,81 +245,92 @@ } else return alloc; } -#ifndef C_URANDOM -/* it's very weak */ -#include -unsigned int cl_rndnum(unsigned int max) +static char *get_tmp_template(const char *dir) { - struct timeval tv; + char *res; - gettimeofday(&tv, (struct timezone *) 0); - srand(tv.tv_usec+clock()); - - return rand() % max; + if (dir == NULL) { + dir = getenv("TMPDIR"); + if (dir == NULL) { +#ifdef P_tmpdir + dir = P_tmpdir; +#else + dir = "/tmp"; +#endif + } + } + res = malloc(strlen(dir) + 14 + 1); + if (res == NULL) { + return NULL; + } + strcpy(res, dir); + strcat(res, "/clamav.XXXXXX"); + return res; } -#else -unsigned int cl_rndnum(unsigned int max) +char *cl_mkdtemp(const char *dir) { - FILE *fd; - unsigned int generated; - char *byte; - int size; + char *res; + int save_errno; - - if((fd = fopen("/dev/urandom", "rb")) == NULL) { - cli_errmsg("!Can't open /dev/urandom.\n"); - return -1; + res = get_tmp_template(dir); + if (!res) { + return NULL; } - - byte = (char *) &generated; - size = sizeof(generated); - do { - int bread; - bread = fread(byte, 1, size, fd); - size -= bread; - byte += bread; - } while(size > 0); - - fclose(fd); - return generated % max; + if (mkdtemp(res)) { + return res; + } + save_errno = errno; + free(res); + errno = save_errno; + return NULL; } -#endif -/* it uses MD5 to avoid potential races in tmp */ -char *cl_gentemp(const char *dir) + +int cl_mkstemp(const char *dir, char **fname) { - char *name, *mdir, *tmp; - unsigned char salt[32]; - int cnt=0, i; - struct stat foo; - - if(!dir) - mdir = "/tmp"; - else - mdir = (char *) dir; - - name = (char*) cli_calloc(strlen(mdir) + 1 + 16 + 1, sizeof(char)); - if(name == NULL) { - cli_dbgmsg("cl_gentemp('%s'): out of memory\n", dir); - return NULL; + char *res; + int fd; + int save_errno; + + res = get_tmp_template(dir); + if (!res) { + return -1; } - cnt += sprintf(name, "%s/", mdir); + fd = mkstemp(res); + if (fd == -1) { + save_errno = errno; + free(res); + errno = save_errno; + return -1; + } + *fname = res; + return fd; +} - do { - for(i = 0; i < 32; i++) - salt[i] = cl_rndnum(255); - - tmp = cl_md5buff(salt, 32); - strncat(name, tmp, 16); - free(tmp); - } while(stat(name, &foo) != -1); - return(name); +int cl_mkftemp(const char *dir, char **fname, FILE **fp) +{ + int fd; + int save_errno; + + fd = cl_mkstemp(dir, fname); + if (fd == -1) { + return -1; + } + *fp = fdopen(fd, "wb+"); + if (*fp == NULL) { + save_errno = errno; + free(*fname); + errno = save_errno; + return -1; + } + return 0; } + int cli_rmdirs(const char *dirname) { DIR *dd; diff -ru clamav-0.67.old/libclamav/scanners.c clamav-0.67/libclamav/scanners.c --- clamav-0.67.old/libclamav/scanners.c 2004-03-06 18:06:25.000000000 -0800 +++ clamav-0.67/libclamav/scanners.c 2004-03-06 18:08:31.000000000 -0800 @@ -29,6 +29,7 @@ #include #include #include +#include #ifdef CL_THREAD_SAFE # include @@ -607,9 +608,9 @@ #endif /* generate the temporary directory */ - dir = cl_gentemp(tmpdir); - if(mkdir(dir, 0700)) { - cli_errmsg("ScanOLE2 -> Can't create temporary directory %s\n", dir); + dir = cl_mkdtemp(tmpdir); + if(dir == NULL) { + cli_errmsg("ScanOLE2 -> Can't create temporary directory in %s: %s\n", tmpdir, strerror(errno)); return CL_ETMPDIR; } @@ -727,9 +728,9 @@ #endif /* generate the temporary directory */ - dir = cl_gentemp(tmpdir); - if(mkdir(dir, 0700)) { - cli_errmsg("ScanMail -> Can't create temporary directory %s\n", dir); + dir = cl_mkdtemp(tmpdir); + if(dir == NULL) { + cli_errmsg("ScanMail -> Can't create temporary directory in %s: %s\n", tmpdir, strerror(errno)); return CL_ETMPDIR; } diff -ru clamav-0.67.old/sigtool/sigtool.c clamav-0.67/sigtool/sigtool.c --- clamav-0.67.old/sigtool/sigtool.c 2004-02-15 03:44:09.000000000 -0800 +++ clamav-0.67/sigtool/sigtool.c 2004-03-06 18:10:02.000000000 -0800 @@ -38,6 +38,7 @@ #include #include #include +#include #include "options.h" #include "others.h" @@ -101,16 +102,11 @@ exit(13); } - if((fname = cl_gentemp(".")) == NULL) { - mprintf("!Can't generate temporary file name.\n"); + if (cl_mkftemp(".", &fname, &wd) == -1) { + mprintf("!Can't generate temporary file name: %s\n", strerror(errno)); exit(1); } - if((wd = fopen(fname, "wb")) == NULL) { - mprintf("!Can't create temporary file %s\n", fname); - exit(14); - } - fseek(rd, start, SEEK_SET); size = end - start; @@ -144,16 +140,11 @@ exit(13); } - if((fname = cl_gentemp(".")) == NULL) { - mprintf("!Can't generate temporary file name.\n"); + if(cl_mkftemp(".", &fname, &wd) == -1) { + mprintf("!Can't generate temporary file name: %s\n", strerror(errno)); exit(1); } - if((wd = fopen(fname, "wb+")) == NULL) { - mprintf("!Can't create temporary file %s\n", fname); - exit(14); - } - while((bytes = fread(buffer, 1, FILEBUFF, rd)) > 0) fwrite(buffer, 1, bytes, wd); @@ -419,16 +410,14 @@ sprintf(signame, "%s.sig", f); if(fileinfo(signame, 1) != -1) { mprintf("File %s exists.\n", signame); - free(signame); - signame = cl_gentemp("."); + exit(15); } bsigname = (char *) mcalloc(strlen(f) + 10, sizeof(char)); sprintf(bsigname, "%s.bsig", f); if(fileinfo(bsigname, 1) != -1) { mprintf("File %s exists.\n", bsigname); - free(bsigname); - bsigname = cl_gentemp("."); + exit(15); } if((wd = fopen(signame, "wb")) == NULL) { @@ -487,6 +476,8 @@ time_t timet; struct tm *brokent; struct cl_cvd *oldcvd = NULL; + int tempfd; + /* build a tar.gz archive * we need: COPYING and {viruses.db, viruses.db2}+ @@ -528,7 +519,12 @@ } } - tarfile = cl_gentemp("."); + tempfd = cl_mkstemp(".", &tarfile); + if (tempfd == -1) { + mprintf("!Can't create temporary tarfile: %s\n", strerror(errno)); + exit(1); + } + close(tempfd); switch(fork()) { case -1: @@ -556,8 +552,12 @@ exit(1); } - gzfile = cl_gentemp("."); - if((gz = gzopen(gzfile, "wb")) == NULL) { + tempfd = cl_mkstemp(".", &gzfile); + if (tempfd == -1) { + mprintf("!Can't open temporary file: %s\n", strerror(errno)); + exit(1); + } + if((gz = gzdopen(tempfd, "wb")) == NULL) { mprintf("!Can't open file %s to write.\n", gzfile); exit(1); } @@ -844,9 +844,9 @@ tmpdir = "/tmp"; #endif - dir = cl_gentemp(tmpdir); - if(mkdir(dir, 0700)) { - mprintf("!listdb(): Can't create temporary directory %s\n", dir); + dir = cl_mkdtemp(tmpdir); + if(dir == NULL) { + mprintf("!listdb(): Can't create temporary directory in %s: %s\n", tmpdir, strerror(errno)); free(buffer); fclose(fd); return -1; @@ -858,11 +858,9 @@ /* start */ - tmp = cl_gentemp(tmpdir); - if((tmpd = fopen(tmp, "wb+")) == NULL) { - mprintf("!listdb(): Can't create temporary file %s\n", tmp); + if (cl_mkftemp(tmpdir, &tmp, &tmpd) == -1) { + mprintf("!listdb(): Can't create temporary file in %s: %s\n", tmpdir, strerror(errno)); free(dir); - free(tmp); free(buffer); fclose(fd); return -1;