Re: [PATCH] Make fsfs survive stale NFS file handles

Malcolm Rowe <malcolm-svn-dev@farside.org.uk> writes:

> > + * You must initialize err to SVN_NO_ERROR, as these macros will never
> > + * touch it if the platform has no ESTALE.
>
> (You must also initialise it to SVN_NO_ERROR because otherwise the
> svn_error_clear() statement at the start will try to free a random
> pointer).

Fixed.

> Reasoning about the behaviour of these macros is pretty hard, though,
> especially as they contain 'continue' statements. Does the benefit
> of code de-duplication really outweigh the cost of understanding them?
> I'm not sure.

They seem simple enough to me, but of course i'm too close to
them. I don't care much one way or the other, i was just trying
to avoid duplication (and perhaps going too far).

> > +/* Long enough to hold: "<svn_revnum_t> <node id> <copy id>\0"
> > + * 19 bytes for svn_revnum_t (long)
>
> That's 10 bytes for a long, 19 bytes would be for a hypothetical 64-bit
> revision number.

I'm running 64-bit systems here, so don't i have a 64-bit value?
Ah, i see more talk about this down-thread. I'll clarify here
that i'm allowing for a 64-bit value.

> > + Allocations are performed from POOL. */
>
> "performed in" is the normal terminology.

I just copied it from another place in this file. Fixed.

> The rest of the patch looks fine.

Thanks.

--
Eric Gillespie <*> epg@pretzelnet.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 5 18:41:30 2007