[Gnu-arch-users] Re: Error handling

[Aaron Bentley]

On Wed, 2003-12-17 at 14:57, Tom Lord wrote:
[snip issue description]
> 
>     > Would you accept a patch that
> 
>     > 1. causes fs_revision_type() to return 0 on success, -1 on failure, and
>     > removes error message (and the same for pfs_revision_type).
> 
>     > 2. moves the error message and exit(2) to arch_revision_type()
> 
>     > 3. adds arch_revision_exists(), which takes the same parameters as
>     > arch_revision_type, but returns 0 if the archive is accessible and -1 if
>     > it is not
> 
>     > 4. changes the arch_revision_type() call in build_revision() into
>     > arch_revision_exists(), and reports "no such revision" if it returns -1

>   
>   (b) if you want to wait a week or so, you can make the change just
>       once to archive-pfs.c only.

Okay, I'll hold off for a week or so

> * arch_build_revision is the wrong place for this test

[snip explanation]

>   Therefore, the better fix here is to call arch_revision_exists from
>   various cmd-*.c files and give the "no such revision" message from
>   there.
> 
>   I would suggest: 
> 
>       use step 1 of your plan, modulo the issue with
>         archive-fs.c vs archive-pfs.c
> 
>       use steps 2 and 3 of your plan as is.
> 
>         add a convenience function, arch_check_for_revision (archive.c
>         is an ok place to add it), which takes as arguments a "struct
>         arch_archive *" and a revision name -- and either returns 
>         normally or exits with a "no such revision" error.   It should
>         call arch_revision_exists.
> 
>       Modify, at least, cmd-get.c.   Just before the call to 
>         arch_build_revision, put a call to arch_check_for_revision.
> 
>       Feel free to similarly modify other cmd-*.c files or to leave
>         those for the next person with a similar itch.   In making 
>         additional modifications there is a slight "art" to it:  to
>         avoid having to connect to an archive just to call
>         arch_check_for_revision -- to connect only in those places
>         which already connect.
> 
> 
> Thank you, by the way, for a lucid description of the change you are
> thinking of.

The codebase is almost entirely unknown to me, and it would have been
bad to start work if I hadn't.

- (p)fs_revision_type() might have been used more widely than I
believed.  
- As it turned out arch_build_revision() *was* the wrong place for the
error handling.

Aaron

-- 
Aaron Bentley
Director of Technology
PanoMetrics, Inc.