[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#325010: marked as done (fileutl.cc error handling necessarily broken)

Your message dated Thu, 13 Aug 2015 13:27:23 +0200
with message-id <20150813112723.GA5939@crossbow>
and subject line Re: fileutl.cc error handling necessarily broken
has caused the Debian Bug report #325010,
regarding fileutl.cc error handling necessarily broken
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
325010: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=325010
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: apt
Version: 0.6.40.1

In fileutl.cc, we see this:

 unsigned long FileFd::Size()
 {
    struct stat Buf;
    if (fstat(iFd,&Buf) != 0)
       return _error->Errno("fstat","Unable to determine the file size");
    return Buf.st_size;
 }

This is wrong, because _error->Errno is supposed to be used like that
in functions which return a boolean success code, not in functions
which return a value.  The interface to ::Size does not permit any
error returns and is therefore wrong: it should return boolean like
most of the other functions, and return the size through a reference
argument (like, say, FTPConn::Size).

I don't supply a patch because (a) it would be large and quickly
outdated (b) it's an easy job to fix the declaration of ::Size and
then have the compiler find all of the callers.

Thanks,
Ian.

--- End Message ---
--- Begin Message --- 
On Thu, Aug 25, 2005 at 03:47:18PM +0100, Ian Jackson wrote:
> In fileutl.cc, we see this:
> 
>  unsigned long FileFd::Size()
>  {
>     struct stat Buf;
>     if (fstat(iFd,&Buf) != 0)
>        return _error->Errno("fstat","Unable to determine the file size");
>     return Buf.st_size;
>  }

Nowadays the function is implemented differently with multiple exists
for different compressors – specifically it returns 0 in error cases,
adds to the error stack as before and sets the error flag on the FileFd
– which is the best we can do about it without breaking the API.

Given that there is pratically no different between an empty file and
a non-existent file for us in terms of reading stuff from it, that seems
to be okayish and is consistent with other methods with similar APIs
like LastModifiedTime() which returns 0 on error as well.

After all, checking the errorstate of the file isn't an unknown pattern
in the C-world in the end… aka: Closing as done.


Best regards

David Kalnischkies

Attachment: signature.asc
Description: Digital signature


--- End Message ---
Reply to: