--exclude-from works but "exclude from" in rsyncd.conf doesn't ?
jw at pegasys.ws
Fri Jun 27 10:05:04 EST 2003
On Thu, Jun 26, 2003 at 12:51:03PM -0700, Wayne Davison wrote:
> I'm finally getting back to finishing up the testing for my patch of the
> "exclude from" handling. The main thing I had on my list was checking
> that --delete-excluded worked right, prompted by your comments:
> Sounds good. I've also improved the args to make_file(). Here's the
> latest patch:
> I think this one is ready to be committed. If anyone disagrees, let me
It's been almost two months so i'm even rustier on you patch
that you could be. I'll jot these down as they occur to me.
Some may be just niggling.
I notice some code formatting changes, that does tend to
obscure things a tiny bit.
| - add_exclude_file(p, 1, 1);
| + add_exclude_file(&server_exclude_list, p, 1, 1);
I know it isn't your patch adding it but these literals are
really more opaque than the code deserves. I'm thinking
we/i should replace them with #defines that indicate
meaning. As i glance through your patch i haven't a clue what
either of those 1s mean and i know this is only the tip of
the iceberg. There are flag arguments of 1, 0 and -1 all
through the code.
| if (verbose >= 2)
| rprintf(FINFO, "%s %s %s because of pattern %s%s\n",
| ent->include ? "including" : "excluding",
| - S_ISDIR(st->st_mode) ? "directory" : "file",
| + name_is_dir ? "directory" : "file",
| name, ent->pattern,
| ent->directory ? "/" : "");
We probably shouldn't be reporting to the client that we
have matched a server-side pattern. I'd say a TODO style
comment would be OK for now while this is is testing.
| - if (check_exclude_file(f, fname, &st))
| + if (check_exclude_file(fname, S_ISDIR(st.st_mode) != 0, exclude_level))
Is != 0 necessary here? I can see using S_ISDIR(),
!S_ISDIR(), or as a unlikely possibliltiy !!S_ISDIR() if
you absolutely need the results of a logical operator.
Maybe it is just me but i see no sense to != 0 unless you
are inverting a test for 0 in an arithmatic just as you
might test for != 6. I'm sure the compiler will optimize it
away but it is still ugly.
| + int check_exclude(struct exclude_struct **list, char *name, int name_is_dir)
| + return 0 && list && name && name_is_dir;
Maybe i'm having a brain-fart but how does this ever return
anything but 0?
The patch looks like it contains some unrelated refactoring
work in util.c. Can we isolate that out please?
I haven't given the patch quite the attention it probably
deserves but i trust my earlier assesment.
J.W. Schultz Pegasystems Technologies
email address: jw at pegasys.ws
Remember Cernan and Schmitt
More information about the rsync