I committed:
> + if (c->authz_read_func)
> + {
> + SVN_ERR (c->authz_read_func (&allowed, c->target_root, target_path,
> + c->authz_read_baton, pool));
> + if (!allowed)
> + return absent_file_or_dir (c, dir_baton, edit_path, tgt_kind, pool);
> + }
Karl wrote;
> I'd declare 'allowed' inside the if's body. Right now, a reader can
> easily fear that 'allowed' might be used uninitialized later. Plus,
> it's slightly easier to add code later which *does* use it
> uninitialized, if one doesn't read the conditional structure
> carefully.
It took me a while to figure out that you meant that people might
think "allowed" is used later on in the function; I initially thought
you meant that people might think "allowed" isn't necessarily
initialized when used in the if statement. Moving the declaration
doesn't make that any more or less likely.
At any rate, there's no reason for programmers to spend any time
thinking about uninitialized variables. The compiler will find them.
Compilers can't be perfect about it, but gcc always errors on the side
of warnings you inappropriately, rather than failing to warn you
appropriately.
I can understand wanting to declare variables in inner scopes so that
the reader can more easily see when the variable is no longer
relevant. But I think it's more important not to break up code with
variable declarations. I hate reading code and having my internal
monologue go: "Okay, if there's an authorization function, we... okay,
here are some variable declarations... er, damn, where was I? Oh,
right, if there's an authorization function, we call it with these
arguments, and if it says no, we..."
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 27 00:15:31 2003