[openbeos] Re: [Open-beos-cvs]CVS: current/src/add-ons/print/transports/usb_port print_transport.cpp,1.4,1.5

I saw this checkin, and am concenred about the symantics of InitCheck() here.

I believe that InitCheck() everywhere else should be returning a status_t.

It returns a bool here. This could be the cause of major future bugs, as people will likely be checking against B_OK (which is equivalent to false, or 0), and not know why InitCheck() is failing.

Might I suggest

        return fFile > -1 ? B_OK : B_ERROR;

or ever caching the error value in fFile or another var and returning that. It seems a shame to throw away perfectly good error values as the code currently does.


Alan

Philippe Houdoin wrote: 
Update of /cvsroot/open-beos/current/src/add-ons/print/transports/usb_port
In directory 
sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv25424/src/add-ons/print/transports/usb_port

Modified Files:
print_transport.cpp Log Message:
Follow the InitCheck() model.

Index: print_transport.cpp
CVS Browser:
http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/open-beos/current/src/add-ons/print/transports/usb_port/print_transport.cpp
===================================================================
RCS file: /cvsroot/open-beos/current/src/add-ons/print/transports/usb_port/print_transport.cpp,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -d -r1.4 -r1.5
--- print_transport.cpp 8 Jun 2004 22:59:21 -0000 1.4
+++ print_transport.cpp 4 Aug 2004 16:39:09 -0000 1.5
@@ -50,7 +50,7 @@
UsbPort(BDirectory* printer, BMessage* msg);
~UsbPort();
- bool IsOk() { return fFile > -1; }
+ bool InitCheck() { return fFile > -1; }
ssize_t Read(void* buffer, size_t size);
ssize_t Write(const void* buffer, size_t size);
@@ -64,13 +64,11 @@
BDataIO* instanciate_transport(BDirectory* printer, BMessage* msg) {
UsbPort* transport = new UsbPort(printer, msg);
- if (transport->IsOk()) {
- if (msg)
- msg->what = 'okok';
+ if (transport->InitCheck())
return transport;
- } else {
- delete transport; return NULL;
- }
+
+ delete transport; + return NULL;
}
@@ -111,6 +109,8 @@
return;

// Fill up the message
+ msg->what = 'okok';
+
msg->AddBool("bidirectional", bidirectional);
msg->AddString("device_id", device_id);
@@ -131,10 +131,8 @@
UsbPort::~UsbPort()
{
- if (IsOk()) {
+ if (fFile > -1)
close(fFile);
- fFile = -1;
- }
}




-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
Open-beos-cvs mailing list
Open-beos-cvs@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/open-beos-cvs


Other related posts: