Vadik Mironov wrote:
Good day, everybody!
In debug mode I encounter massive stack corruption, with the help of global
call_stack array. The only reason to this is plain return in function or no
EDBUG() macro at the start of the function.
I never use the EDBUG macro stuff myself, so it may very well be broken.
I guess it can be useful when beginning to figure out what the code
actually does at all :)
You want to replace some macros implementing a few lines of code with
some fairly huge ones. I suggest to move the code into some functions
and let the macros call these, something like this:
#define EDBUG(l, x) EDebugBegin(l, x)
#define EDBUG_RETURN(x) \
do { EDebugEnd(__FILE__, __LINE__); return (x); } while(0)
#define EDBUG_RETURN_ \
do { EDebugEnd(__FILE__, __LINE__); return; } while(0)
Furthermore
- This patch doesn't apply, I guess due to lines broken by your mailer.
It is better to send patches as attachments.
- Does the patch do anything useful at all? The first change removes the
closing "*/" from a comment ?!?
So i suggest to join DBGCHECK from events.c and EDBUG macro, here is the
patch:
--- ./etalon/e16/e/src/E.h 2004-08-31 21:24:39.000000000 +0400
+++ ./hacked/e16/e/src/E.h 2004-09-07 11:52:40.959028008 +0400
@@ -128,44 +128,72 @@
/* and being able to trace E for profiling/optimisation purposes (which */
/* believe it or not I'm actually doing) */
-/* #define DEBUG 1 */
+/*#define DEBUG 1
#ifdef DEBUG
-extern int call_level;
-extern char *call_stack[1024];
-
+extern int call_level ;
+extern char *call_stack[1024] ;
+extern int save_stack ;
#endif
+
#ifdef DEBUG
-#define EDBUG(l,x) \
-{ \
- call_stack[call_level] = x; \
- call_level++; \
-}
+ #define EDBUG(l,x)\
+ {\
+ save_stack = call_level + 1 ;\
+ fprintf(stderr,"Entered %s with call level %i\n",x,call_level);\
+ call_stack[call_level] = x ;\
+ call_level++ ;\
+ }
#else
-#define EDBUG(l,x) \
-;
+ #define EDBUG(l,x)\
+ ;
#endif
-#ifdef DEBUG
-#define EDBUG_RETURN(x) \
-{ \
- call_level--; \
- return (x); \
-}
-#define EDBUG_RETURN_ \
-{ \
- call_level--; \
- return; \
-}
+#ifdef DEBUG
+ #define EDBUG_RETURN(x)\
+ {\
+ if (save_stack != call_level)\
+ {\
+ fprintf (stderr, "Unstack error with saved value %i :
[",save_stack);\
+ for (save_stack = 0; save_stack < call_level; save_stack++)\
+ {\
+ fprintf (stderr, "%s%s", save_stack ? ", " : "",
call_stack[save_stack]);\
+ }\
+ fprintf (stderr, "]\n") ;\
+ fprintf (stderr, "file %s , line %i\n",__FILE__,__LINE__);\
+ save_stack = call_level ;\
+ }\
+ call_level-- ;\
+ fprintf(stderr,"Exited %s with call level
%i\n",call_stack[call_level],call_level);\
+ return (x) ;\
+ }
+
+ #define EDBUG_RETURN_\
+ {\
+ if (save_stack != call_level)\
+ {\
+ fprintf (stderr, "Unstack error with saved value %i :
[",save_stack);\
+ for (save_stack = 0; save_stack < call_level; save_stack++)\
+ {\
+ fprintf (stderr, "%s%s", save_stack ? ", " : "",
call_stack[save_stack]);\
+ }\
+ fprintf (stderr, "]\n") ;\
+ fprintf (stderr, "file %s , line %i\n",__FILE__,__LINE__);\
+ save_stack = call_level ;\
+ }\
+ call_level-- ;\
+ fprintf(stderr,"Exited %s with call level
%i\n",call_stack[call_level],call_level);\
+ return ;\
+ }
#else
-#define EDBUG_RETURN(x) \
-{ \
- return (x); \
-}
-#define EDBUG_RETURN_ \
-{ \
- return; \
-}
+ #define EDBUG_RETURN(x)\
+ {\
+ return (x) ;\
+ }
+ #define EDBUG_RETURN_\
+ {\
+ return ;\
+ }
#endif
#ifdef HAVE_SNPRINTF
Another issue is the lack of brackets around this macro in the code, there are
blocks in macro, but still the code should work under any circumstances, here
is the patch,
You are right. However, I think the macros are bad, not the code. If you
do as shown above (the do{}while(0) thing) I think the problem is
solved. This should avoid making ~1600 unnecessary code changes :)
also i add some commentary (i believe, if everybody each time
when he/she submit something, add commentary to an arbitrary function, very
soon we get very well documented code :-)):
Again, you are right. E16 is not excessively commented :) I haven't
found it worth while to break that tradition. I'm in favor of
self-explanatory code anyway, except when documenting a public API.
--- file-e.c 2004-07-13 03:33:15.000000000 +0400
+++ file.c 2004-09-07 12:05:18.838812768 +0400
@@ -101,21 +101,46 @@
}
}
+/**
+ * Check whether file with a name s is a regular file.
+ *
+ * Parameters :
+ * s - pointer to a null terminated file name.
+ *
+ * Return value :
+ * 0 - if a given name corresponds to a non regular file.
+ * 1 - if a given name corresponds to a regular file.
+ */
int
isfile(const char *s)
-{
- struct stat st;
-
- EDBUG(9, "isfile");
- if ((!s) || (!*s))
- EDBUG_RETURN(0);
- if (stat(s, &st) < 0)
- EDBUG_RETURN(0);
- if (S_ISREG(st.st_mode))
- EDBUG_RETURN(1);
- EDBUG_RETURN(0);
+{
+ struct stat st ;
+
+ EDBUG(9, "isfile") ;
+
+ /*if name is NULL or '\0' */
+ if ((!s) || (!*s))
+ {
+ EDBUG_RETURN(0) ;
+ }
+
+ if (stat(s, &st) < 0)
+ {
+ if (DEBUG)
+ {
+ perror("isfile") ;
+ }
+ EDBUG_RETURN(0) ;
+ }
+
+ if (S_ISREG(st.st_mode))
+ {
+ EDBUG_RETURN(1) ;
+ }
+ EDBUG_RETURN(0) ;
}
--- snip ---
/Kim
-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
enlightenment-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
