geoff@utcsstat.UUCP (Geoff Collyer) (05/08/84)
I don't think I've seen any of these bugs posted yet. I put sortactive() in our readnews and got loud screams from users who were suddenly seeing groups they had never read before in front of all their old favourites. If you want groups not in the user's .newsrc yet in the active file to appear *last*, add the marked lines in the middle of sortactive() (in rfuncs.c) table[nlines].rcpos = findrcline(line); + if (table[nlines].rcpos == -1) /* not in .newsrc */ + table[nlines].rcpos = LINES; /* huge, sort to the end */ table[nlines].actpos = actpos; Elsewhere in rfuncs.c, I found some more sscanf's whose return values were unchecked (yawn). I suppose I should 'egrep scanf *.c' some day and fix them once and for all. Line numbers are approximate. 35c35,38 < sscanf(afline, "%s %ld", bfr, &nngsize); --- > if (sscanf(afline, "%s %ld", bfr, &nngsize) < 2) { > bfr[0] = '\0'; > nngsize = 0; > } 257c256,259 < sscanf(afline, "%s %ld", bfr, &nngsize); --- > if (sscanf(afline, "%s %ld", bfr, &nngsize) < 2) { > bfr[0] = '\0'; > nngsize = 0; > } There's a latent bug in selectng() in rfuncs.c which only showed up here when a user without an options line in his .newsrc tried `readnews -x' and was told `No news.'; he knew this was too good to be true and reported it. 75,82c78,83 < if (!xflag) { < i = findrcline(name); < if (i >= 0) { < if (index(rcline[i], '!')) { < groupdir[0] = 0; < return 1; < } < sprintf(rcbuf, "%s,%ld", rcline[i], ngsize+1); --- > if (xflag || (i = findrcline(name)) < 0) > sprintf(rcbuf, "ng: %ld", ngsize+1); > else { > if (index(rcline[i], '!')) { > groupdir[0] = 0; > return 1; 84,85c85 < else < sprintf(rcbuf, "ng: %ld", ngsize+1); --- > sprintf(rcbuf, "%s,%ld", rcline[i], ngsize+1); The problem is that under -x, the old code didn't initialise rcbuf. If the above diffs aren't enough, here's the new code fragment: if (xflag || (i = findrcline(name)) < 0) sprintf(rcbuf, "ng: %ld", ngsize+1); else { if (index(rcline[i], '!')) { groupdir[0] = 0; return 1; } sprintf(rcbuf, "%s,%ld", rcline[i], ngsize+1); } Our readr.c didn't ever print the `Last article' prompt and I fixed it by rearranging readr() (I also fixed a debugging printf format): if (getnextart(FALSE)) { /* no more articles */ if (sigtrap || !news) break; /* don't print Last article */ /* else fall through */ } else { #ifdef DEBUG printf("after getnextart, fp %x, pos %ld, bit %d, group '%s', filename '%s'\n", fp, ftell(fp), bit, groupdir, filename); #endif strcpy(goodone, filename); if (pflag || lflag || eflag) { /* This code should be gotten rid of */ if (sigtrap) { qfflush(ofp); fprintf(ofp, "\n"); cdump(ofp); _exit(0); /* kludge! drop when qfflush works */ return; } clear(bit); nextbit(); if (fp) { fclose(fp); fp = NULL; } continue; } } for (; ; ) { If you are using sortactive(), you'll probably want to read the user's .newrc, even under -x, so that you can sort the active file into .newsrc order. In readnews.c, I changed a few lines and added two: 225c225,226 < if (!xflag && (rcfp = xfopen(newsrc, "r"))) { --- > /* must read newsrc under xflag for sortactive() */ > if ((rcfp = xfopen(newsrc, "r"))) { sortactive(); + if (xflag) + line = -1; /* ignore newsrc by forgetting its contents */ That's all for today. Geoff Collyer, U. of Toronto Computing Services
rees@apollo.UUCP (05/11/84)
The trouble with Geoff's fix to readr() is that it now prompts you
on the last article even if you have one of the non-interactive
flags on (like -e, my favorite flag). Of course the original code
wasn't right either.
It seems to me that you want to either do the interactive stuff,
in the inner for() loop, or the non-interactive stuff. Here is
my version of readr(). I tested the simple cases but can't claim
to fully understand this code even now.
readr()
{
int gotart;
#ifdef DEBUG
fprintf(stderr, "readr()\n");
#endif
if (aflag) {
if (*datebuf) {
if ((atime = cgtdate(datebuf)) == -1)
xerror("Cannot parse date string");
} else
atime = 0L;
}
if (pflag && ignoring())
ignorenews = TRUE;
if (uflag)
time(&timelastsaved);
ofp = stdout;
if (cflag && coptbuf[0] != '\0') {
umask(022);
mktemp(outfile); /* get "unique" file name */
ofp = xfopen(outfile, "w");
umask(N_UMASK);
cflag = FALSE;
pflag = TRUE;
}
/* loop reading articles. */
fp = NULL;
obit = -1;
nextng();
for ( ;; ) {
gotart = !getnextart(FALSE);
if (!news)
break;
#ifdef DEBUG
printf("after getnextart, fp %x, pos %ld, bit %d, group '%s', filename '%s'\n",
fp, ftell(fp), bit, groupdir, filename);
#endif
if (gotart)
strcpy(goodone, filename);
if (pflag || lflag || eflag) {
/* This code should be gotten rid of */
if (sigtrap) {
qfflush(ofp);
fprintf(ofp, "\n");
cdump(ofp);
_exit(0); /* kludge! drop when qfflush works */
return;
}
if (gotart) {
clear(bit);
nextbit();
} else
break;
if (fp) {
fclose(fp);
fp = NULL;
}
continue;
}
for (; ; ) {
char *pp;
#ifdef SIGCONT
int (*ocont)();
#endif
sigtrap = FALSE;
if (!cflag) {
if (rfq)
fprintf(ofp, "Last article. [qfr] ");
else
fprintf(ofp, "(%d lines) More? [ynq] ", NLINES(h, fp));
} else
fprintf(ofp, "? ");
fflush(ofp);
bptr = lbuf;
#ifdef SIGCONT
ocont = signal(SIGCONT, catchcont);
#endif
pp = fgets(bptr, BUFLEN, stdin);
#ifdef SIGCONT
signal(SIGCONT, ocont);
#endif
if (pp != NULL)
break;
if (!sigtrap)
return;
#ifdef SIGCONT
if (sigtrap != SIGCONT)
#endif
fprintf(ofp, "\n");
}
nstrip(bptr);
while (*bptr == ' ' || *bptr == '\t')
bptr++;
i = command();
if (i)
break;
}
if (!news)
fprintf(stderr, "No news.\n");
cout(ofp);
}geoff@utcsstat.UUCP (Geoff Collyer) (05/15/84)
I'm sorry that I broke the non-interactive modes of readnews with my latest batch of changes. I still don't know what -e does, because it isn't documented (in readnews(1) anyway), and I don't have enough spare time to decrypt the source. I guess this is a penalty for trying to provide five or six user interfaces in one monolithic, lumbering hulk. I think Mr. Rees's fix is cleaner than mine. I certainly don't fully understand readr() and I doubt that anyone does. The news swillware is full of gotos, continues, four-page functions and global variables. I'd like to know why the main, essential, core logic of readnews is scattered across three functions in three source files.
joe@fluke.UUCP (05/16/84)
Geoff's (and Jim's) fix both have the undesirable side effect that if
you try to do anything "interesting" at the "Last article" prompt,
readnews will probably dump core. The reason is that many of the
options in the big switch in command() try to close(fp). If you are at
the "Last article" prompt, fp == NULL and fclose immediately gets an
illegal reference and dumps. The fix is to enclose all occurrences of
the two lines
fclose(fp);
fp = NULL;
in command() with
if (fp != NULL) {
<as above>
}
Note that there are already several cases which have taken into account
that fp might be NULL. I think there are six or seven unprotected
fclose's.
Also, if you unsubscribe to the last newsgroup in the active file,
readnews doesn't print the Last article prompt. I haven't looked into
this one very closely yet. Maybe I'll install Jim's version of readr
first.
/Joe