geoff@utcsstat.UUCP (Geoffrey Collyer) (01/05/84)
I have recently found and fixed two readnews 2.10.1 bugs that cause core dumps on v7, in the undigestifier H command and the function findngsize. The undigestifier (invoked by the d command to readnews) takes an H subcommand which called hprint with an extra argument *in the middle of the argument list*, which eventually caused a Bus Error in fprintf. Diffs of digest.c follow: 201c201 < hprint(h, !cflag, ofp, 1); --- > hprint(h, ofp, 1); 209c209 < hprint(h, !cflag, ofp, 0); --- > hprint(h, ofp, 0); (Lines numbers are approximate; actual line numbers may vary.) Two days later, after I had read roughly one hundred articles, readnews took a Memory Fault and dumped core, for the third time in three days. Of course, by now I was getting used to it. I dug through the core dump, and since I had wisely not stripped my readnews binary determined that the global long ngsize had been trashed, causing selectn() to try to zero far, far beyond the end of bitmap[]. This happened shortly after I had deleted the most recent round of dead news groups from /usr/lib/news/active, which naturally made me suspect findngsize(). This is the vile swill that used to be findngsize (in rfuncs.c): /* * Figure out the number of the largest article in newsgroup ng, * and return that value. */ long findngsize(ng) char *ng; { FILE *af; long s; char buf[100], n[100]; af = xfopen(ACTIVE, "r"); while (fgets(buf, sizeof buf, af)) { sscanf(buf, "%s %ld", n, &s); if (strcmp(n, ng) == 0) { fclose(af); return s; } } return 0; } This foul, evil-smelling excuse for a function failed to close ACTIVE (/usr/lib/news/active) if the newsgroup ng was not found *and* returned utter trash (whatever was on the stack) if ACTIVE was malformed, since it failed to check that sscanf actual read two items from buf. I replaced it with this function: long findngsize(ng) char *ng; { FILE *af; long s, ret = 0; char buf[100], n[100]; af = xfopen(ACTIVE, "r"); while (fgets(buf, sizeof buf, af) != NULL) if (sscanf(buf, "%s %ld", n, &s) == 2 && strcmp(n, ng) == 0) { ret = s; break; } fclose(af); return ret; } There is a further minor bug in inews.c. At the start of localize(), the line actfp = fopen(ACTIVE, "r+"); appears, and actfp is never tested to see if fopen succeeded. This particular bug hasn't bitten me, but if inews ever can't read and write the ACTIVE file, it will start fprintfing on NULL, which can produce bizarre results. fcloseing NULL produces a core dump on utcsstat (a v7 system). There are many other such unchecked system calls and function calls throughout B news and trying to chase them all down would be an endless task. System calls *do* fail, even when *you* think they shouldn't or can't or won't. I wish people would assume the worst when writing programs: system calls will always fail, at the most inconvenient times, when you are least able to deal with them, for reasons that you can't foresee. If you at least try to do something reasonable, rather than just ``knowing'' that nothing can possibly go wrong, your program won't go berserk when faced with the unusual! All right, officer, I'll go peacefully... Geoff Collyer, U. of Toronto
rees@apollo.UUCP (Jim Rees) (01/09/84)
There is a further minor bug in inews.c.
At the start of localize(), the line
actfp = fopen(ACTIVE, "r+");
appears, and actfp is never tested to see if fopen succeeded.
Yes, I fixed that one here a while ago. There isn't any really
sane thing to do at that point, so I made it an xerror().
In the same routine, there are two places where the function
will return without fcloseing ACTIVE, either. Try submitting an
article with the following "Newsgroups:" line to rnews:
Newsgroups: general,a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z
Each of those invalid groups will leave a hanging FILE pointer, and
you will run out after just a few of them. Granted this is a
pathological case, but I don't think it is wise to leave these
things open after they are needed.
The fix is to put a "fclose(actfp)" before each of the two "return FALSE"
statements.