aal_assert()

aal_assert()

[Andrew Clausen]

Hi all,

In reiser4progs, aal_assert() has an argument for an assertian
identifier, so calls look like this:

        aal_assert("umka-889", name != NULL, return NULL);

(Where umka == Yura Umanets :)

Isn't file + line number + version number sufficient to identify
the spot in the code?  If all of these are printed out in error
messages, then users' bug reports should give sufficient information
to trace the problem.

So, I think debug.[ch] should be changed like this:

--- debug.h~	2002-12-29 08:18:42.000000000 +1100
+++ debug.h	2002-12-29 08:17:38.000000000 +1100
@@ -20,9 +20,9 @@
 */
 #ifdef __GNUC__
 
-#define aal_assert(hint, cond, action)	\
+#define aal_assert(cond, action)	\
     do {				\
-    	if (!__assert(hint, cond,	\
+    	if (!__assert(cond,		\
 	   #cond,			\
 	    __FILE__,			\
 	    __LINE__,			\
@@ -30,13 +30,13 @@
 	{				\
 	    action;			\
 	}				\
-    } while (0);
+    } while (0)
 
 #else
 
-#define aal_assert(hint, cond, action)	\
+#define aal_assert(cond, action)	\
     do {				\
-	if (!__assert(hint, cond,	\
+	if (!__assert(cond,		\
 	    #cond,			\
 	    "unknown",			\
 	    0,				\
@@ -44,17 +44,17 @@
 	{				\
 	    action;			\
 	}				\
-    } while (0);
+    } while (0)
 
 #endif
 
 #else
 
-#define aal_assert(hint, cond, action) while (0) {}
+#define aal_assert(cond, action) while (0) {}
 
 #endif
 
-extern int __assert(char *hint, int cond, char *text, char *file, 
+extern int __assert(int cond, char *text, char *file, 
 		    int line, char *function);
 
 #endif

--- debug.c~	2002-12-29 08:15:27.000000000 +1100
+++ debug.c	2002-12-29 08:16:41.000000000 +1100
@@ -18,7 +18,6 @@
    aal_assert().
 */
 int __assert(
-	char *hint,	     /* person owner of assert */
 	int cond,	     /* condition of assertion */
 	char *text,	     /* text of the assertion */
 	char *file,	     /* source file assertion was failed in */
@@ -34,8 +33,8 @@
 	   file, line and function assertion was failed in.
 	*/ 
 	return (aal_exception_throw(EXCEPTION_BUG, EXCEPTION_IGNORE | EXCEPTION_CANCEL,
-				    "%s: Assertion (%s) at %s:%d in function %s() failed.",
-				    hint, text, file, line, function) == EXCEPTION_IGNORE);
+				    "Assertion (%s) at %s:%d of version %s in function %s() failed.",
+				    text, file, line, VERSION, function) == EXCEPTION_IGNORE);
 }
 
 #endif

Cheers,
Andrew

Re: aal_assert()

[Yury Umanets]

Andrew Clausen wrote:

>Hi all,
>
>In reiser4progs, aal_assert() has an argument for an assertian
>identifier, so calls look like this:
>
>        aal_assert("umka-889", name != NULL, return NULL);
>
>(Where umka == Yura Umanets :)
>
>Isn't file + line number + version number sufficient to identify
>the spot in the code?  If all of these are printed out in error
>messages, then users' bug reports should give sufficient information
>to trace the problem.
>
>So, I think debug.[ch] should be changed like this:
>
>--- debug.h~	2002-12-29 08:18:42.000000000 +1100
>+++ debug.h	2002-12-29 08:17:38.000000000 +1100
>@@ -20,9 +20,9 @@
> */
> #ifdef __GNUC__
> 
>-#define aal_assert(hint, cond, action)	\
>+#define aal_assert(cond, action)	\
>     do {				\
>-    	if (!__assert(hint, cond,	\
>+    	if (!__assert(cond,		\
> 	   #cond,			\
> 	    __FILE__,			\
> 	    __LINE__,			\
>@@ -30,13 +30,13 @@
> 	{				\
> 	    action;			\
> 	}				\
>-    } while (0);
>+    } while (0)
> 
> #else
> 
>-#define aal_assert(hint, cond, action)	\
>+#define aal_assert(cond, action)	\
>     do {				\
>-	if (!__assert(hint, cond,	\
>+	if (!__assert(cond,		\
> 	    #cond,			\
> 	    "unknown",			\
> 	    0,				\
>@@ -44,17 +44,17 @@
> 	{				\
> 	    action;			\
> 	}				\
>-    } while (0);
>+    } while (0)
> 
> #endif
> 
> #else
> 
>-#define aal_assert(hint, cond, action) while (0) {}
>+#define aal_assert(cond, action) while (0) {}
> 
> #endif
> 
>-extern int __assert(char *hint, int cond, char *text, char *file, 
>+extern int __assert(int cond, char *text, char *file, 
> 		    int line, char *function);
> 
> #endif
>
>--- debug.c~	2002-12-29 08:15:27.000000000 +1100
>+++ debug.c	2002-12-29 08:16:41.000000000 +1100
>@@ -18,7 +18,6 @@
>    aal_assert().
> */
> int __assert(
>-	char *hint,	     /* person owner of assert */
> 	int cond,	     /* condition of assertion */
> 	char *text,	     /* text of the assertion */
> 	char *file,	     /* source file assertion was failed in */
>@@ -34,8 +33,8 @@
> 	   file, line and function assertion was failed in.
> 	*/ 
> 	return (aal_exception_throw(EXCEPTION_BUG, EXCEPTION_IGNORE | EXCEPTION_CANCEL,
>-				    "%s: Assertion (%s) at %s:%d in function %s() failed.",
>-				    hint, text, file, line, function) == EXCEPTION_IGNORE);
>+				    "Assertion (%s) at %s:%d of version %s in function %s() failed.",
>+				    text, file, line, VERSION, function) == EXCEPTION_IGNORE);
> }
> 
> #endif
>
>Cheers,
>Andrew
>
>
>  
>
Actually there was the code earlier like you have proposed. And it was 
changed to the current state due to the following reasons:

* there are two developers working on reiser4progs currentry (Vitaly 
Fertman and Yury Umanets). And our parts is much different. So, each of 
us should work on own code.
* there is not neccessary an assertion is corect and only the author can 
decide is it correct.
* NAMESYS has some development policies. And hints is the one of them.


-- 
Yury Umanets

Re: aal_assert()

[Andrew Clausen]

On Fri, Jan 10, 2003 at 10:22:43AM +0300, Yura Umanets wrote:
> Actually there was the code earlier like you have proposed. And it was 
> changed to the current state due to the following reasons:
> 
> * there are two developers working on reiser4progs currentry (Vitaly 
> Fertman and Yury Umanets). And our parts is much different. So, each of 
> us should work on own code.
> * there is not neccessary an assertion is corect and only the author can 
> decide is it correct.

Well, that decision is made by the developers, not the users.
Presumebly, you know who wrote what code.  If an assertion is
contentious (and 99.9% of them aren't), you should document *why*,
anyway.

> * NAMESYS has some development policies. And hints is the one of them.

Why?

Cheers,
Andrew

Re: aal_assert()

[Yury Umanets]

Andrew Clausen wrote:

>On Fri, Jan 10, 2003 at 10:22:43AM +0300, Yura Umanets wrote:
>  
>
>>Actually there was the code earlier like you have proposed. And it was 
>>changed to the current state due to the following reasons:
>>
>>* there are two developers working on reiser4progs currentry (Vitaly 
>>Fertman and Yury Umanets). And our parts is much different. So, each of 
>>us should work on own code.
>>* there is not neccessary an assertion is corect and only the author can 
>>decide is it correct.
>>    
>>
>
>Well, that decision is made by the developers, not the users.
>
Decision is made by developers in debug and test stage.

>Presumebly, you know who wrote what code.  If an assertion is
>contentious (and 99.9% of them aren't), you should document *why*,
>anyway.
>
I should document why an assert is contentious? I think there should not 
exist contentious assersions. Assertion is correct or not.

>
>  
>
>>* NAMESYS has some development policies. And hints is the one of them.
>>    
>>
>
>Why?
>
I don't know :)) Probably Nikita will tell you better. Hi is the old 
resident of NAMESYS.

>
>Cheers,
>Andrew
>
>
>
>  
>


-- 
Yury Umanets

Re: aal_assert()

[Andrew Clausen]

On Thu, Jan 09, 2003 at 02:30:08PM +0300, Nikita Danilov wrote:
> These "message ids" are traditional way reiserfs code uses to identify
> errors (probably because early during development preprocessor features
> were unknown) and Hans feels himself attached to them emotionally. :-)

Well, I'll give some arguments against this scheme:

(1) it's more code, more work, more space, etc.  Programmers are
minimalists, and adding extra stuff feels dirty.

(2) "ids" are the Wrong Way to identify things... things should
be identified by their essence, not by tags.  This is the whole
OO vs relational database flamewar.  Relational databases
(the "by essense/being, not tag") won, game over.

You don't query things by tags, but by what they are.

I know this is all quite subjective... I just wanted to provide
some emotional arguments as well as rational ones *grin*

Another thing: I'd like to see libaal reused in all file system
tool implementations.  That's my main motivation for cleaning it
up.  My first target is ntfs...

Cheers,
Andrew

Re: aal_assert()

[Yury Umanets]

Andrew Clausen wrote:

>On Thu, Jan 09, 2003 at 02:30:08PM +0300, Nikita Danilov wrote:
>  
>
>>These "message ids" are traditional way reiserfs code uses to identify
>>errors (probably because early during development preprocessor features
>>were unknown) and Hans feels himself attached to them emotionally. :-)
>>    
>>

>Well, I'll give some arguments against this scheme:
>
>(1) it's more code, more work, more space, etc.  Programmers are
>minimalists, and adding extra stuff feels dirty.
>
>(2) "ids" are the Wrong Way to identify things... things should
>be identified by their essence, not by tags.  This is the whole
>OO vs relational database flamewar.  Relational databases
>(the "by essense/being, not tag") won, game over.
>
>You don't query things by tags, but by what they are.
>
>I know this is all quite subjective... I just wanted to provide
>some emotional arguments as well as rational ones *grin*
>

>
>Another thing: I'd like to see libaal reused in all file system
>tool implementations.  That's my main motivation for cleaning it
>up.  My first target is ntfs...
>
It is more reasonable. Probably Hans will agree with you because of this.

>
>Cheers,
>Andrew
>
>
>
>  
>


-- 
Yury Umanets