Fritjof Bornebusch <[email protected]> writes:
> Hi tech,
Hello, Fritjof,
> there is a dirty if statement in rlog.c, that checks if there is a valid
> locker, state or writer and returns if not.
> With help from jca - thanks for that - I removed the dirty if statement and
> check for valid data in the sections.
Well, you're welcome. But this is exactly the diff I wrote and sent
you, except for the declarations of found_(locker|state|writer), which
you moved to a wider scope...
Anyway, I see two possible reasons for your change:
- you want their scope to match the scope of largv, sargv, wargv. This
is a good reason, but another way to reach this would be to narrow the
scope of said variables. (diff below)
- you prefer to have all variables declared at function scope. In this
case I don't know why you would keep the "author" variable at block
scope as in my diff...
I generally prefer that variables have the narrowest possible scope,
some people don't, in the end I don't care much.
> I tested it and it behaves like the previous one with the dirty if - I hope I
> didn't missed something.
The rationale for this diff is that if -l, -s and -w are just filters
that are AND'ed to see if we should print a revision, then as soon as
a condition is false we don't need to test the others.
I did not have the time to look at other implementations, but at least
this looks consistent with our documentation. I did not try to
fully disentangle the previous "monster" condition to see if the
behaviors matched.
If people have more experience with RCS and can comment on this diff,
thanks in advance. ;)
Index: rlog.c
===================================================================
RCS file: /cvs/src/usr.bin/rcs/rlog.c,v
retrieving revision 1.67
diff -u -p -p -u -r1.67 rlog.c
--- rlog.c 7 Jan 2014 14:08:16 -0000 1.67
+++ rlog.c 26 May 2014 12:23:13 -0000
@@ -426,80 +426,85 @@ rlog_file(const char *fname, RCSFILE *fi
static void
rlog_rev_print(struct rcs_delta *rdp)
{
- int i, found;
struct tm t;
- char *author, numb[RCS_REV_BUFSZ], *fmt, timeb[RCS_TIME_BUFSZ];
- struct rcs_argvector *largv, *sargv, *wargv;
+ char numb[RCS_REV_BUFSZ], *fmt, timeb[RCS_TIME_BUFSZ];
struct rcs_branch *rb;
struct rcs_delta *nrdp;
-
- i = found = 0;
- author = NULL;
+ int i;
/* -l[lockers] */
if (lflag == 1) {
- if (rdp->rd_locker != NULL)
- found++;
+ int found_locker = 0;
if (llist != NULL) {
+ struct rcs_argvector *largv;
+
/* if locker is empty, no need to go further. */
if (rdp->rd_locker == NULL)
return;
+
largv = rcs_strsplit(llist, ",");
for (i = 0; largv->argv[i] != NULL; i++) {
if (strcmp(rdp->rd_locker, largv->argv[i])
== 0) {
- found++;
+ found_locker = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(largv);
- }
+ } else if (rdp->rd_locker != NULL)
+ found_locker = 1;
+
+ if (!found_locker)
+ return;
}
/* -sstates */
if (slist != NULL) {
+ struct rcs_argvector *sargv;
+ int found_state = 0;
+
sargv = rcs_strsplit(slist, ",");
for (i = 0; sargv->argv[i] != NULL; i++) {
if (strcmp(rdp->rd_state, sargv->argv[i]) == 0) {
- found++;
+ found_state = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(sargv);
+
+ if (!found_state)
+ return;
}
/* -w[logins] */
if (wflag == 1) {
+ int found_writer = 0;
+
if (wlist != NULL) {
+ struct rcs_argvector *wargv;
+
wargv = rcs_strsplit(wlist, ",");
for (i = 0; wargv->argv[i] != NULL; i++) {
- if (strcmp(rdp->rd_author, wargv->argv[i])
- == 0) {
- found++;
+ if (!strcmp(rdp->rd_author, wargv->argv[i])) {
+ found_writer = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(wargv);
} else {
+ char *author;
+
if ((author = getlogin()) == NULL)
err(1, "getlogin");
if (strcmp(rdp->rd_author, author) == 0)
- found++;
+ found_writer = 1;
}
- }
- /* XXX dirty... */
- if ((((slist != NULL && wflag == 1) ||
- (slist != NULL && lflag == 1) ||
- (lflag == 1 && wflag == 1)) && found < 2) ||
- (((slist != NULL && lflag == 1 && wflag == 1) ||
- (slist != NULL || lflag == 1 || wflag == 1)) && found == 0))
- return;
+ if (!found_writer)
+ return;
+ }
printf("%s\n", REVSEP);
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
