www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.syserror could use a bit of tidying up

reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Looking at the std.syserror module, there are a few things about it that 
don't seem right:

(a) It is a class consisting of nothing but a single static function. 
Obviously whoever wrote it was thinking in Java (or maybe C#).

(b) At the moment it covers only Windows error codes.  It should be 
versioned for portability.  (Consequently, at the moment FileExceptions 
thrown on Linux are going to have the wrong messages....)

(c) It requires the error code to be passed in.  How about a function 
that actually interrogates the error code in the platform-appropriate 
way and then picks out the right message?


Two proposed solutions:

1. Deprecate the pointless Java idiom and use module-level functions. 
Implement the function aforementioned in (c).  Using an out parameter, 
the caller would be able to receive both the error code and the message.

Pro: Can be used in arbitrary exception classes.

Con: By itself, applications will not be able to tell at a glance 
whether an arbitrary exception class contains an error code.

2. Turn SysError into an exception class.  The constructor would take in 
an arbitrary string to use in the message (such as the filename as 
std.file is implemented, or an indication of what it failed to do), 
interrogate the system error code, and build the exception message from 
this information.  Exception classes such as FileException and 
StreamException would then derive from SysError.  (Would it make sense 
to call it SysException instead while we're at it?)

Pros: Would make exceptions involving system error codes easier to 
implement.  The SysError class would have the overall semantics of an 
exception containing a system error code; maybe some applications could 
make use of this.

Con: Classes that already derive from some other class cannot also 
derive from SysError.  Hence the idea doesn't get on well with the 
concept of using a base class to cover a certain category of errors that 
may or may not contain a system error code.

What do people think we should do?

Stewart.

-- 
My e-mail is valid but not my primary mailbox.  Please keep replies on 
the 'group where everyone may benefit.
Apr 12 2005
next sibling parent reply "Regan Heath" <regan netwin.co.nz> writes:
------------ou4F7tPNZMxOWBdHsIkiFU
Content-Type: text/plain; format=flowed; delsp=yes; charset=iso-8859-15
Content-Transfer-Encoding: 8bit

On Tue, 12 Apr 2005 11:13:05 +0100, Stewart Gordon <smjg_1998 yahoo.com>  
wrote:
 Looking at the std.syserror module, there are a few things about it that  
 don't seem right:

 (a) It is a class consisting of nothing but a single static function.  
 Obviously whoever wrote it was thinking in Java (or maybe C#).

Indeed. It does seem a bit redundant.. unless.. what if you want to construct it when the error occurs but save it for later? i.e. save both the error code and the resulting string.
 (b) At the moment it covers only Windows error codes.  It should be  
 versioned for portability.  (Consequently, at the moment FileExceptions  
 thrown on Linux are going to have the wrong messages....)

I have a version that calls strerror on Linux. I believe it works. (but have not tested it on a Linux machine)
 (c) It requires the error code to be passed in.  How about a function  
 that actually interrogates the error code in the platform-appropriate  
 way and then picks out the right message?

The problem I have with this idea is.. for example Socket error codes are returned by the WSAGetLastError function, normal error codes by GetLastError, both sets of codes can be passed to my version of syserror.d and it will generate the correct error message. So, in any situation where the error code does not come directly from the GetLastError function (on windows) or errno (on linux) but can still have a message generated by the FormatMessage (on windows) and strerror (on linux) functions you're stuck.
 Two proposed solutions:

 1. Deprecate the pointless Java idiom and use module-level functions.  
 Implement the function aforementioned in (c).  Using an out parameter,  
 the caller would be able to receive both the error code and the message.

 Pro: Can be used in arbitrary exception classes.

 Con: By itself, applications will not be able to tell at a glance  
 whether an arbitrary exception class contains an error code.

I don't like this, for reasons voiced above.
 2. Turn SysError into an exception class.  The constructor would take in  
 an arbitrary string to use in the message (such as the filename as  
 std.file is implemented, or an indication of what it failed to do),  
 interrogate the system error code, and build the exception message from  
 this information.  Exception classes such as FileException and  
 StreamException would then derive from SysError.  (Would it make sense  
 to call it SysException instead while we're at it?)

 Pros: Would make exceptions involving system error codes easier to  
 implement.  The SysError class would have the overall semantics of an  
 exception containing a system error code; maybe some applications could  
 make use of this.

 Con: Classes that already derive from some other class cannot also  
 derive from SysError.  Hence the idea doesn't get on well with the  
 concept of using a base class to cover a certain category of errors that  
 may or may not contain a system error code.

But, if we combine this option with exception chaining (see Ben's post about this), we can throw an exception, and pass SysError as the 'cause' of that exception, right?
 What do people think we should do?

Option #2 with error chaining. Attached is my (not perfect) version of syserror.d Regan ------------ou4F7tPNZMxOWBdHsIkiFU Content-Disposition: attachment; filename=syserror.d Content-Type: application/octet-stream; name=syserror.d Content-Transfer-Encoding: 8bit module lib.syserror; private import std.string; private import std.c.windows.windows; private import std.c.stdarg; extern (Windows) { DWORD FormatMessageA( DWORD dwFlags, LPCVOID lpSource, DWORD dwMessageId, DWORD dwLanguageId, LPTSTR lpBuffer, DWORD nSize, va_list *Arguments ); static uint FORMAT_MESSAGE_ALLOCATE_BUFFER = 0x00000100; static uint FORMAT_MESSAGE_IGNORE_INSERTS = 0x00000200; static uint FORMAT_MESSAGE_FROM_STRING = 0x00000400; static uint FORMAT_MESSAGE_FROM_HMODULE = 0x00000800; static uint FORMAT_MESSAGE_FROM_SYSTEM = 0x00001000; static uint FORMAT_MESSAGE_ARGUMENT_ARRAY = 0x00002000; static uint FORMAT_MESSAGE_MAX_WIDTH_MASK = 0x000000FF; WORD MAKELANGID(WORD p, WORD s) { return (((cast(WORD)s) << 10) | cast(WORD)p); } WORD PRIMARYLANGID(WORD lgid) { return (cast(WORD)lgid & 0x3ff); } WORD SUBLANGID(WORD lgid) { return (cast(WORD)lgid >> 10); } static uint LANG_NEUTRAL = 0x00; static uint SUBLANG_DEFAULT = 0x01; alias HGLOBAL HLOCAL; HLOCAL LocalFree(HLOCAL hMem); } extern (C) char *strerror(int); class SysError { static char[] msg(uint errcode) { char[] text; version(Windows) { if (errcode == ERROR_NOT_ENOUGH_MEMORY) text = "not enough memory"; else { LPVOID lpMsgBuf; DWORD r; r = FormatMessageA( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, null, errcode, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language cast(LPTSTR)&lpMsgBuf, 0, null ); /* Remove \r\n from error string */ if (r >= 2) r -= 2; text = (cast(char *)lpMsgBuf)[0..r].dup; LocalFree(cast(HLOCAL)lpMsgBuf); } } else { if (errcode == ENOMEM) text = "not enough memory"; else { char* pemsg; uint r; pemsg = strerror(errcode); r = strlen(pemsg); /* Remove \r\n from error string */ if (pemsg[r-1] == '\n') r--; if (pemsg[r-1] == '\r') r--; text = pemsg[0..r].dup; } } return text; } } ------------ou4F7tPNZMxOWBdHsIkiFU--
Apr 12 2005
next sibling parent "Regan Heath" <regan netwin.co.nz> writes:
FYI the attached syserror.d is intended for "public domain".

On Tue, 12 Apr 2005 23:52:58 +1200, Regan Heath <regan netwin.co.nz> wrote:
 On Tue, 12 Apr 2005 11:13:05 +0100, Stewart Gordon <smjg_1998 yahoo.com>
 wrote:
 Looking at the std.syserror module, there are a few things about it that
 don't seem right:

 (a) It is a class consisting of nothing but a single static function.
 Obviously whoever wrote it was thinking in Java (or maybe C#).

Indeed. It does seem a bit redundant.. unless.. what if you want to construct it when the error occurs but save it for later? i.e. save both the error code and the resulting string.
 (b) At the moment it covers only Windows error codes.  It should be
 versioned for portability.  (Consequently, at the moment FileExceptions
 thrown on Linux are going to have the wrong messages....)

I have a version that calls strerror on Linux. I believe it works. (but have not tested it on a Linux machine)
 (c) It requires the error code to be passed in.  How about a function
 that actually interrogates the error code in the platform-appropriate
 way and then picks out the right message?

The problem I have with this idea is.. for example Socket error codes are returned by the WSAGetLastError function, normal error codes by GetLastError, both sets of codes can be passed to my version of syserror.d and it will generate the correct error message. So, in any situation where the error code does not come directly from the GetLastError function (on windows) or errno (on linux) but can still have a message generated by the FormatMessage (on windows) and strerror (on linux) functions you're stuck.
 Two proposed solutions:

 1. Deprecate the pointless Java idiom and use module-level functions.
 Implement the function aforementioned in (c).  Using an out parameter,
 the caller would be able to receive both the error code and the message.

 Pro: Can be used in arbitrary exception classes.

 Con: By itself, applications will not be able to tell at a glance
 whether an arbitrary exception class contains an error code.

I don't like this, for reasons voiced above.
 2. Turn SysError into an exception class.  The constructor would take in
 an arbitrary string to use in the message (such as the filename as
 std.file is implemented, or an indication of what it failed to do),
 interrogate the system error code, and build the exception message from
 this information.  Exception classes such as FileException and
 StreamException would then derive from SysError.  (Would it make sense
 to call it SysException instead while we're at it?)

 Pros: Would make exceptions involving system error codes easier to
 implement.  The SysError class would have the overall semantics of an
 exception containing a system error code; maybe some applications could
 make use of this.

 Con: Classes that already derive from some other class cannot also
 derive from SysError.  Hence the idea doesn't get on well with the
 concept of using a base class to cover a certain category of errors that
 may or may not contain a system error code.

But, if we combine this option with exception chaining (see Ben's post about this), we can throw an exception, and pass SysError as the 'cause' of that exception, right?
 What do people think we should do?

Option #2 with error chaining. Attached is my (not perfect) version of syserror.d Regan

Apr 12 2005
prev sibling next sibling parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Regan Heath wrote:
 On Tue, 12 Apr 2005 11:13:05 +0100, Stewart Gordon
 <smjg_1998 yahoo.com>  wrote:
 
 Looking at the std.syserror module, there are a few things about 
 it that don't seem right:
 
 (a) It is a class consisting of nothing but a single static 
 function.  Obviously whoever wrote it was thinking in Java (or 
 maybe C#).

Indeed. It does seem a bit redundant.. unless.. what if you want to construct it when the error occurs but save it for later? i.e. save both the error code and the resulting string.

It certainly is redundant until this idea is implemented. But if you'd like to implement it, go ahead. It leads towards option 2 anyway. <snip>
 (c) It requires the error code to be passed in.  How about a 
 function that actually interrogates the error code in the 
 platform-appropriate way and then picks out the right message?

The problem I have with this idea is.. for example Socket error codes are returned by the WSAGetLastError function, normal error codes by GetLastError, both sets of codes can be passed to my version of syserror.d and it will generate the correct error message.

Then we could hold on the two functions. One is the msg function we have now, and the other is the one that interrogates the error code by the 'usual' means. They could become two constructor forms when option 2 is coded up.
 So, in any situation where the error code does not come directly 
 from the GetLastError function (on windows) or errno (on linux) but 
 can still have a message generated by the FormatMessage (on 
 windows) and strerror (on linux) functions you're stuck.
 
 Two proposed solutions:
 
 1.  Deprecate the pointless Java idiom and use module-level 
 functions.  Implement the function aforementioned in (c).  Using 
 an out parameter, the caller would be able to receive both the 
 error code and the message.
 
 Pro: Can be used in arbitrary exception classes.
 
 Con: By itself, applications will not be able to tell at a glance 
 whether an arbitrary exception class contains an error code.

I don't like this, for reasons voiced above.

Don't like what exactly? <snip>
 2.  Turn SysError into an exception class.  But, if we combine this 
 option with exception chaining (see Ben's post about this), we can 
 throw an exception, and pass SysError as the 'cause' of that 
 exception, right?
 
 What do people think we should do?

Option #2 with error chaining.

I see.... Stewart. -- My e-mail is valid but not my primary mailbox. Please keep replies on the 'group where everyone may benefit.
Apr 12 2005
parent reply "Regan Heath" <regan netwin.co.nz> writes:
------------xXEFwz6T51TebN2njhbW2y
Content-Type: text/plain; format=flowed; delsp=yes; charset=iso-8859-15
Content-Transfer-Encoding: 8bit

Assimilating this thread I have produced the attached code.
Comments?

Regan
------------xXEFwz6T51TebN2njhbW2y
Content-Disposition: attachment; filename=syserror.d
Content-Type: application/octet-stream; name=syserror.d
Content-Transfer-Encoding: 8bit

module lib.syserror;

private import std.string;
private import std.c.stdarg;

version(Windows) {
	import std.c.windows.windows;
	
	extern (Windows) {
		DWORD FormatMessageA(
			DWORD dwFlags,
			LPCVOID lpSource,
			DWORD dwMessageId,
			DWORD dwLanguageId,
			LPTSTR lpBuffer,
			DWORD nSize,
			va_list *Arguments
		);

		static uint FORMAT_MESSAGE_ALLOCATE_BUFFER = 0x00000100;
		static uint FORMAT_MESSAGE_IGNORE_INSERTS  = 0x00000200;
		static uint FORMAT_MESSAGE_FROM_STRING     = 0x00000400;
		static uint FORMAT_MESSAGE_FROM_HMODULE    = 0x00000800;
		static uint FORMAT_MESSAGE_FROM_SYSTEM     = 0x00001000;
		static uint FORMAT_MESSAGE_ARGUMENT_ARRAY  = 0x00002000;
		static uint FORMAT_MESSAGE_MAX_WIDTH_MASK  = 0x000000FF;

		WORD MAKELANGID(WORD p, WORD s)  { return (((cast(WORD)s) << 10) |
cast(WORD)p); }
		WORD PRIMARYLANGID(WORD lgid)    { return (cast(WORD)lgid & 0x3ff); }
		WORD SUBLANGID(WORD lgid)        { return (cast(WORD)lgid >> 10); }

		static uint LANG_NEUTRAL = 0x00;
		static uint SUBLANG_DEFAULT = 0x01;

		alias HGLOBAL HLOCAL;

		HLOCAL LocalFree(HLOCAL hMem);
		
		uint ERROR_NOT_ENOUGH_MEMORY = 8;
	}
}

extern (C) char *strerror(int);

class SystemError : Error
{
	uint errorCode;
	
	this(uint code)
	{		
		super(systemError(code));
		errorCode = code;
	}
}

char[] systemError()
{
	version(Windows) return systemError(GetLastError());
	else return systemError(getErrno());
}

char[] systemError(uint errcode)
{
	char[] text;
	
	version(Windows)
	{
		if (errcode == ERROR_NOT_ENOUGH_MEMORY) text = "not enough memory";
		else {
			LPVOID lpMsgBuf;
			DWORD r;

			r = FormatMessageA( 
				FORMAT_MESSAGE_ALLOCATE_BUFFER | 
				FORMAT_MESSAGE_FROM_SYSTEM | 
				FORMAT_MESSAGE_IGNORE_INSERTS,
				null,
				errcode,
				MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
				cast(LPTSTR)&lpMsgBuf,
				0,
				null 
			);
			/* Remove \r\n from error string */
			if (r >= 2) r -= 2;
			text = (cast(char *)lpMsgBuf)[0..r].dup;
			LocalFree(cast(HLOCAL)lpMsgBuf);
		}
	}
	else
	{
		if (errcode == ENOMEM) text = "not enough memory";
		else {
			char* pemsg;
			uint r;

			pemsg = strerror(errcode);
			r = strlen(pemsg);
			/* Remove \r\n from error string */
			if (pemsg[r-1] == '\n') r--;
			if (pemsg[r-1] == '\r') r--;
			text = pemsg[0..r].dup;
		}
	}

	return text;
}


------------xXEFwz6T51TebN2njhbW2y--
Apr 12 2005
next sibling parent reply "Ben Hinkle" <ben.hinkle gmail.com> writes:
"Regan Heath" <regan netwin.co.nz> wrote in message 
news:opso43rwqc23k2f5 nrage.netwin.co.nz...
 Assimilating this thread I have produced the attached code.
 Comments?

Very nice! One question: did you subclass Error to get "the class that allows chaining" or for some other reason? I'm guessing you did for the chaining. If Error is merged with Exception and the Error class goes away what do you think of keeping the name SysError but subclassing Exception? I think the name SysError is ok since the OS functions are called GetLastError and strerror and errorCode etc etc. So to summarize, user code would look something like: version (Windows) { handle = SomeOSFunction(...); if (handle == INVALID_HANDLE_VALUE) { throw new SomeException("Something failed",new SysError(GetLastError())); } } else { result = someOSFunction(...); if (result == error) { throw new SomeException("Something failed",new SysError(getErrno())); } }
Apr 12 2005
parent reply "Regan Heath" <regan netwin.co.nz> writes:
On Tue, 12 Apr 2005 19:14:16 -0400, Ben Hinkle <ben.hinkle gmail.com>  
wrote:
 "Regan Heath" <regan netwin.co.nz> wrote in message
 news:opso43rwqc23k2f5 nrage.netwin.co.nz...
 Assimilating this thread I have produced the attached code.
 Comments?

Very nice!

Thank you.
 One question: did you subclass Error to get "the class that
 allows chaining" or for some other reason? I'm guessing you did for the
 chaining.

Yes.
 If Error is merged with Exception and the Error class goes away
 what do you think of keeping the name SysError but subclassing Exception?

Sounds good.
 I think the name SysError is ok since the OS functions are called  
 GetLastError and strerror and errorCode etc etc.

Indeed. Or SystemError if people want something a little more verbose, I don't mind either myself.
 So to summarize, user code would look something like:

 version (Windows) {
   handle = SomeOSFunction(...);
   if (handle == INVALID_HANDLE_VALUE) {
     throw new SomeException("Something failed",new
 SysError(GetLastError()));
   }
 } else {
   result = someOSFunction(...);
   if (result == error) {
     throw new SomeException("Something failed",new SysError(getErrno()));
   }
 }

Exactly. It's maybe important to note that, for example, your average programmer might never need/see SysError itself as we will likely do things like: try { BufferedFile f = new BufferedFile("a.txt",FileMode.In); } catch (FileNotFound e) { } in Phobos, where FileNotFound is thrown by a line resembling: throw new FileNotFound("Failed to read("~file~")",new SysError(GetLastError())); Regan
Apr 12 2005
parent "Ben Hinkle" <ben.hinkle gmail.com> writes:
 try {
   BufferedFile f = new BufferedFile("a.txt",FileMode.In);
 } catch (FileNotFound e) {
 }

 in Phobos, where FileNotFound is thrown by a line resembling:

 throw new FileNotFound("Failed to read("~file~")",new 
 SysError(GetLastError()));

Agreed. I've always been annoyed that the exceptions in std.stream don't capture the full error information from the system. Currently I think if a file stream ctor fails it says something like "Could not create or open file" and maybe it doesn't even give the file name. It would be very nice to give as much info as possible.
Apr 12 2005
prev sibling parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Regan Heath wrote:
 Assimilating this thread I have produced the attached code.
 Comments?

I see, except that I don't see: - a SystemError constructor that does the interrogation as per systemError() - any room for a context message - or have you decided this should be left to the class that chains with it whatever that may be? Stewart. -- My e-mail is valid but not my primary mailbox. Please keep replies on the 'group where everyone may benefit.
Apr 13 2005
parent "Regan Heath" <regan netwin.co.nz> writes:
On Wed, 13 Apr 2005 10:14:15 +0100, Stewart Gordon <smjg_1998 yahoo.com>  
wrote:

 Regan Heath wrote:
 Assimilating this thread I have produced the attached code.
 Comments?

I see, except that I don't see: - a SystemError constructor that does the interrogation as per systemError()

That's a good idea.
 - any room for a context message - or have you decided this should be  
 left to the class that chains with it whatever that may be?

Yes. Regan
Apr 14 2005
prev sibling parent "Ben Hinkle" <bhinkle mathworks.com> writes:
 2. Turn SysError into an exception class.

But, if we combine this option with exception chaining (see Ben's post about this), we can throw an exception, and pass SysError as the 'cause' of that exception, right?

That might be pretty good. The SysError never gets thrown itself - it just gets carried around as the cause. I like that better than my reply to Stewart about adding some methods to Exception. It seems cleaner to have the smarts in SysError and add the methods there and attach it to the cause.
Apr 12 2005
prev sibling parent reply "Ben Hinkle" <bhinkle mathworks.com> writes:
"Stewart Gordon" <smjg_1998 yahoo.com> wrote in message 
news:d3g6vh$28ob$1 digitaldaemon.com...
 Looking at the std.syserror module, there are a few things about it that 
 don't seem right:

 (a) It is a class consisting of nothing but a single static function. 
 Obviously whoever wrote it was thinking in Java (or maybe C#).

agreed
 (b) At the moment it covers only Windows error codes.  It should be 
 versioned for portability.  (Consequently, at the moment FileExceptions 
 thrown on Linux are going to have the wrong messages....)

agreed
 (c) It requires the error code to be passed in.  How about a function that 
 actually interrogates the error code in the platform-appropriate way and 
 then picks out the right message?

 Two proposed solutions:

 1. Deprecate the pointless Java idiom and use module-level functions. 
 Implement the function aforementioned in (c).  Using an out parameter, the 
 caller would be able to receive both the error code and the message.

 2. Turn SysError into an exception class.
 What do people think we should do?

I like option 2 except that it feels dangerous to tie the inheritance tree to platform-specific support of error codes or strings. Off the top of my head perhaps SysError needs to get merged with Exception (copied from my proposed Exception in a previous post): class Exception { char[] msg; Object cause; this(char[] msg, Object cause = null); void print(); // print this exception and any causes char[] toString(); // string summarizes this exception bit hasSysCode(out int code); bit hasSysMsg(out char[] msg); } It would be useful to have both a platform-independent message in msg and a platform-dependent message (if any) in hasSysMsg. By default hasSysCode and hasSysMsg return false. I haven't thought too hard about supporting system codes and strings but it would be nice to do cleanly in the exception hierarchy somehow. I hope other people chime in with ideas, too.
Apr 12 2005
parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Ben Hinkle wrote:
<snip>
2. Turn SysError into an exception class.
What do people think we should do?

I like option 2 except that it feels dangerous to tie the inheritance tree to platform-specific support of error codes or strings. Off the top of my head perhaps SysError needs to get merged with Exception (copied from my proposed Exception in a previous post): class Exception { char[] msg; Object cause; this(char[] msg, Object cause = null); void print(); // print this exception and any causes char[] toString(); // string summarizes this exception bit hasSysCode(out int code); bit hasSysMsg(out char[] msg); }

What would cause be under this idea? And the forms of hasSysCode and hasSysMsg don't seem right. Why should we be forced to supply a dummy variable just to see whether a system error code exists? I'm more inclined towards something like bit hasSysCode(); int sysErrorCode(); char[] sysErrorMessage(); The last of these could return null if there is no system error code to translate. Stewart. -- My e-mail is valid but not my primary mailbox. Please keep replies on the 'group where everyone may benefit.
Apr 13 2005
parent "Ben Hinkle" <ben.hinkle gmail.com> writes:
"Stewart Gordon" <smjg_1998 yahoo.com> wrote in message 
news:d3inne$190v$1 digitaldaemon.com...
 Ben Hinkle wrote:
 <snip>
2. Turn SysError into an exception class.
What do people think we should do?

I like option 2 except that it feels dangerous to tie the inheritance tree to platform-specific support of error codes or strings. Off the top of my head perhaps SysError needs to get merged with Exception (copied from my proposed Exception in a previous post): class Exception { char[] msg; Object cause; this(char[] msg, Object cause = null); void print(); // print this exception and any causes char[] toString(); // string summarizes this exception bit hasSysCode(out int code); bit hasSysMsg(out char[] msg); }

What would cause be under this idea?

I prefer Regan's solution. Please ignore my suggestion above - it was made without reading all other other posts.
Apr 13 2005