I updated andygoth-versioned-open and tested, only to find it panics
when opening a repository that doesn't have both
.fossil-settings/allow-symlinks and
.fossil-settings/allow-symlinks.no-warn .

I believe I fixed it, though I would appreciate outside review before I
commit to trunk.

The reason for the panic is the call to historical_version_of_file().
If it can't find the requested file, it panics if errCode is 0 *or
negative*.  I've corrected the comment to reflect this fact.

Maybe we ought to rethink the way historical_version_of_file() handles
return codes.  If errCode is nonpositive, it panics on error.  So if the
caller needs to be able to handle errors, it needs to pass a positive
number.  But which number?  The caller has to guess a value that it
would otherwise never return, which is tough because its return value is
undocumented.

For the moment, it returns whatever value is returned by content_get(),
which is 0, 1, or the return value of content_of_blob().
content_of_blob() returns 0 or 1.  But I had to grovel through the
sources to figure that out, and I wonder if 2 will get used someday.

Searching further, I found that historical_version_of_file() is called
in six places (not counting my branch), and only one of them passes a
positive errCode.  In that case, the value that is passed is 2.  This
all makes me wonder if the errCode parameter should be replaced with one
that explicitly selects between panicking or returning 0 on failure.

I also found an incorrect use of historical_version_of_file().
cat_cmd() passes 0 errCode, then checks if the return value is 0 to
decide whether to print "no such file: %s".  However, it never gets the
chance because historical_version_of_file() panics after printing "file
%s does not exist in check-in:" (the revision was supposed to be
displayed after the colon, but no revision is supplied by cat_cmd()
unless the -r option is used).  I checked in a quick fix, but again more
thought is called for.

-- 
Andy Goth | <andrew.m.goth/at/gmail/dot/com>

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
fossil-dev mailing list
fossil-dev@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/fossil-dev

Reply via email to