All of lore.kernel.org
 help / color / mirror / Atom feed
* aal_assert()
@ 2002-12-31  0:54 Andrew Clausen
  2003-01-10  7:22 ` aal_assert() Yury Umanets
       [not found] ` <15901.23872.24060.68353@laputa.namesys.com>
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Clausen @ 2002-12-31  0:54 UTC (permalink / raw)
  To: reiserfs-list

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: aal_assert()
  2002-12-31  0:54 aal_assert() Andrew Clausen
@ 2003-01-10  7:22 ` Yury Umanets
  2003-01-10 13:36   ` aal_assert() Andrew Clausen
       [not found] ` <15901.23872.24060.68353@laputa.namesys.com>
  1 sibling, 1 reply; 6+ messages in thread
From: Yury Umanets @ 2003-01-10  7:22 UTC (permalink / raw)
  To: Andrew Clausen; +Cc: reiserfs-list

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: aal_assert()
  2003-01-10 13:26   ` aal_assert() Andrew Clausen
@ 2003-01-10 13:19     ` Yury Umanets
  0 siblings, 0 replies; 6+ messages in thread
From: Yury Umanets @ 2003-01-10 13:19 UTC (permalink / raw)
  To: Andrew Clausen; +Cc: Nikita Danilov, reiserfs-list

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: aal_assert()
       [not found] ` <15901.23872.24060.68353@laputa.namesys.com>
@ 2003-01-10 13:26   ` Andrew Clausen
  2003-01-10 13:19     ` aal_assert() Yury Umanets
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Clausen @ 2003-01-10 13:26 UTC (permalink / raw)
  To: Nikita Danilov; +Cc: reiserfs-list

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: aal_assert()
  2003-01-10  7:22 ` aal_assert() Yury Umanets
@ 2003-01-10 13:36   ` Andrew Clausen
  2003-01-10 14:12     ` aal_assert() Yury Umanets
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Clausen @ 2003-01-10 13:36 UTC (permalink / raw)
  To: Yury Umanets; +Cc: reiserfs-list

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: aal_assert()
  2003-01-10 13:36   ` aal_assert() Andrew Clausen
@ 2003-01-10 14:12     ` Yury Umanets
  0 siblings, 0 replies; 6+ messages in thread
From: Yury Umanets @ 2003-01-10 14:12 UTC (permalink / raw)
  To: Andrew Clausen; +Cc: reiserfs-list

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-01-10 14:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-12-31  0:54 aal_assert() Andrew Clausen
2003-01-10  7:22 ` aal_assert() Yury Umanets
2003-01-10 13:36   ` aal_assert() Andrew Clausen
2003-01-10 14:12     ` aal_assert() Yury Umanets
     [not found] ` <15901.23872.24060.68353@laputa.namesys.com>
2003-01-10 13:26   ` aal_assert() Andrew Clausen
2003-01-10 13:19     ` aal_assert() Yury Umanets

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.