From: Stephen T. <st...@sb...> - 2003-02-27 00:35:37
|
I am willing, possible glutton for punishment, to go through xine-lib and convert all assert(...) statements to the following statement: For example, assert(exp); To: if (exp =3D=3D NULL) { printf("<filename>: <function> - ERROR - exp is not defined. Value is NULL\n"); abort(); } Granted it four/five lines extra instead of one but its much more descriptive on what went wrong and where. It is certainly a more descriptive line of text to sent to the developers (namely us) in pinpointing the exact problem area. I am aware that I need to send my work to the libdvdread/libdvdnav developers when I get their files complete. Let me know what the opinion is about this (a.k.a RFC). Stephen --=20 Stephen Torri <st...@sb...> |
From: Mark T. <mb...@do...> - 2003-02-27 02:59:20
|
What about: #define XINE_ASSERT(exp, desc, args...) \ if (!(exp)) { \ printf ("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ __FILE__, __FUNCTION__, __LINE__ ,##args); abort(); } Then you just need to change assert (exp) to XINE_ASSERT (exp, "Reason for failure."); with the added bonus that you can do: XINE_ASSERT (exp, "Too many sprockets, expected %d, got %d.", expected, got); Note that it may be necessary to wrap printf in a ``xine_assertion_failed'' function, as printf is a macro on some architectures. Though this makes the varargs somewhat harder... Regards, Mark. Mark Thomas \ Computing Year 4 \ Imperial College / United Kingdom / What do you mean my birth certificate expired? |
From: Stephen T. <st...@sb...> - 2003-02-27 03:33:22
|
On Wed, 2003-02-26 at 20:58, Mark Thomas wrote: > What about: >=20 > #define XINE_ASSERT(exp, desc, args...) \ > if (!(exp)) { \ > printf ("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ > __FILE__, __FUNCTION__, __LINE__ ,##args); abort(); } On review this is something I understand. > Note that it may be necessary to wrap printf in a > ``xine_assertion_failed'' function, as printf is a macro on some > architectures. Though this makes the varargs somewhat harder... >=20 So do you want to have: #define XINE_ASSERT (exp, desc, args...) \ if (!(exp)) { \ xine_assertion_failed)(exp, desc, args); \ } I am afraid my macro skills are not that good just yet. Stephen --=20 Stephen Torri <st...@sb...> |
From: Mark T. <mb...@do...> - 2003-02-27 10:53:40
|
On Thu, 26 Feb 2003, Stephen Torri wrote: >> Note that it may be necessary to wrap printf in a >> ``xine_assertion_failed'' function, as printf is a macro on some >> architectures. Though this makes the varargs somewhat harder... >> Actually, thinking about it, I'm talking rubbish. The first definition I gave is fine. What you CAN'T do is this: printf ("%d\n", #ifdef SOMETHING 0 #else 1 #endif ); As this will fail on systems where printf is a macro. I'll use the fact that I was up at 2 in the morning as an excuse, I think. >I am afraid my macro skills are not that good just yet. They get better with practice ;-) Though I don't consider myself anywhere near as good as some of my friends: what they can do with the C pre-processor is pure evil. Regards, Mark. Mark Thomas \ Computing Year 4 \ Imperial College / United Kingdom / Don't use a big word when a diminutive one will suffice. |
From: Bastien N. <ha...@ha...> - 2003-02-27 11:05:00
|
On Thu, 2003-02-27 at 00:35, Stephen Torri wrote: > I am willing, possible glutton for punishment, to go through xine-lib > and convert all assert(...) statements to the following statement: > > For example, > > assert(exp); > > To: > > if (exp == NULL) { > printf("<filename>: <function> - ERROR - exp is not > defined. Value is > NULL\n"); > abort(); > } > > Granted it four/five lines extra instead of one but its much more > descriptive on what went wrong and where. It is certainly a more > descriptive line of text to sent to the developers (namely us) in > pinpointing the exact problem area. > > I am aware that I need to send my work to the libdvdread/libdvdnav > developers when I get their files complete. > > Let me know what the opinion is about this (a.k.a RFC). assert are good to 1) spot programming errors 2) spot memory corruptions. I don't think it's a good idea to convert the assert()'s to just plain printf. If the assert are used to bail out when corrupted data is fed to the engine, these parts that use it should be fixed not to use it. I wouldn't think that the assert()'s as used in xine-lib should be replaced by printf()'s, it makes debugging harder. Cheers -- /Bastien Nocera http://hadess.net #2 0x4205a2cc in printf ("Oh my %s\n", preferred_deity) from /lib/i686/libc.so.6 printf ("Oh my %s\n", preferred_deity); Segmentation fault |
From: Stephen T. <st...@sb...> - 2003-02-27 15:20:13
|
On Thu, 2003-02-27 at 05:09, Bastien Nocera wrote: > assert are good to 1) spot programming errors 2) spot memory > corruptions. I don't think it's a good idea to convert the assert()'s to > just plain printf. If the assert are used to bail out when corrupted > data is fed to the engine, these parts that use it should be fixed not > to use it. >=20 > I wouldn't think that the assert()'s as used in xine-lib should be > replaced by printf()'s, it makes debugging harder. We are using the following in place of 'assert()': /** * Provide assert like feature with better description of failure=20 * Thanks to Mark Thomas=20 */ #define XINE_ASSERT(exp, desc, args...) \ if (!(exp)) { \ printf("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ __FILE__, __FUNCTION__, __LINE__, ##args); \ abort(); \ } You get the same functionality as assert() because all assert() does is print out a terse violation message only containing the condition. Afterwards assert() calls abort(). So to allow for more descriptive message we use printf() and abort() together. Stephen --=20 Stephen Torri <st...@sb...> |
From: Bastien N. <ha...@ha...> - 2003-02-27 15:26:58
|
On Thu, 2003-02-27 at 15:20, Stephen Torri wrote: > On Thu, 2003-02-27 at 05:09, Bastien Nocera wrote: > > > assert are good to 1) spot programming errors 2) spot memory > > corruptions. I don't think it's a good idea to convert the assert()'s to > > just plain printf. If the assert are used to bail out when corrupted > > data is fed to the engine, these parts that use it should be fixed not > > to use it. > > > > I wouldn't think that the assert()'s as used in xine-lib should be > > replaced by printf()'s, it makes debugging harder. > > We are using the following in place of 'assert()': > > /** > * Provide assert like feature with better description of failure > * Thanks to Mark Thomas > */ > #define XINE_ASSERT(exp, desc, args...) \ > if (!(exp)) { \ > printf("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ > __FILE__, __FUNCTION__, __LINE__, ##args); \ > abort(); \ > } > > You get the same functionality as assert() because all assert() does is > print out a terse violation message only containing the condition. > Afterwards assert() calls abort(). So to allow for more descriptive > message we use printf() and abort() together. Fair enough, suits me (I guess I didn't read the macro properly). Cheers -- /Bastien Nocera http://hadess.net #2 0x4205a2cc in printf ("Oh my %s\n", preferred_deity) from /lib/i686/libc.so.6 printf ("Oh my %s\n", preferred_deity); Segmentation fault |
From: Stephen T. <st...@sb...> - 2003-02-27 15:36:25
|
On Thu, 2003-02-27 at 09:31, Bastien Nocera wrote: > Fair enough, suits me (I guess I didn't read the macro properly). >=20 > Cheers Cool. I am on the task. I have placed the macro in src/xine-utils/xineutils.h. Its seems to be a safe place that is commonly accessed by a variety of files. Stephen --=20 Stephen Torri <st...@sb...> |
From: Daniel Caujolle-B. <seg...@cl...> - 2003-02-27 16:48:01
|
Hi, Stephen Torri wrote: > On Thu, 2003-02-27 at 09:31, Bastien Nocera wrote: > > >>Fair enough, suits me (I guess I didn't read the macro properly). >> >>Cheers > > > Cool. I am on the task. I have placed the macro in > src/xine-utils/xineutils.h. Its seems to be a safe place that is > commonly accessed by a variety of files. > > Stephen embbed this macro in a do {} while(0) btw. Cheers. -- 73's de Daniel, F1RMB. -=- Daniel Caujolle-Bert -=- seg...@cl... -=- -=- f1...@f1... (AMPR NET) -=- |
From: Guenter B. <bar...@we...> - 2003-02-27 16:51:44
|
hi mark, > What about: > > #define XINE_ASSERT(exp, desc, args...) \ > if (!(exp)) { \ > printf ("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ > __FILE__, __FUNCTION__, __LINE__ ,##args); abort(); } please - no more macros. use a function. thanks, guenter -- "What is the Nature of God?" CLICK...CLICK...WHIRRR...CLICK...=BEEP!= 1 QT. SOUR CREAM 1 TSP. SAUERKRAUT 1/2 CUT CHIVES. STIR AND SPRINKLE WITH BACON BITS. "I've just GOT to start labeling my software..." -- Bloom County |
From: Mark T. <mb...@do...> - 2003-02-28 10:42:43
|
On Thu, 27 Feb 2003, Guenter Bartsch wrote: >hi mark, > >> What about: >> >> #define XINE_ASSERT(exp, desc, args...) \ >> if (!(exp)) { \ >> printf ("%s:%s:%d: assertion `" #exp "' failed. " desc "\n", \ >> __FILE__, __FUNCTION__, __LINE__ ,##args); abort(); } > >please - no more macros. use a function. > Unfortunately, this can't be a function as you can't "stringize" the expression without getting the pre-processor to do it, and you can only determine the file, line and function in-place. I agree, though. If it were possible, I'd much prefer to use an inline functon. Regards, Mark. Mark Thomas \ Computing Year 4 \ Imperial College / United Kingdom / Gotta run, the cat's caught in the printer. |
From: Guenter B. <bar...@we...> - 2003-02-27 16:53:26
|
hallo stephen, > I am aware that I need to send my work to the libdvdread/libdvdnav > developers when I get their files complete. according to http://freshmeat.net/releases/114241/ libdvdread 0.9.4: Changes: [...] All asserts have been removed [...] so maybe it is just a matter of updating xine's copy to the latest version here? cheers, guenter -- "What is the Nature of God?" CLICK...CLICK...WHIRRR...CLICK...=BEEP!= 1 QT. SOUR CREAM 1 TSP. SAUERKRAUT 1/2 CUT CHIVES. STIR AND SPRINKLE WITH BACON BITS. "I've just GOT to start labeling my software..." -- Bloom County |
From: Michael R. <mr...@us...> - 2003-02-27 21:27:06
|
Hi guenter, > according to > > http://freshmeat.net/releases/114241/ > > libdvdread 0.9.4: > > Changes: [...] All asserts have been removed [...] > > so maybe it is just a matter of updating xine's copy to the latest > version here? Wanted to do this anyway. Do I have time until weekend? Btw, I don't think kicking out the assert()s in libdvdnav is necessary. I have watched a lot of DVDs and haven't seen one triggering for ages. But I won't keep anyone from doing it anyway. Patches can be sent to dvd-devel where I will receive them. (But don't call it XINE_ASSERT in libdvdnav ;) Michael -- /* * Should be panic but... (Why are BSD people panic obsessed ??) */ 2.0.38 /usr/src/linux/net/ipv4/ip_fw.c |
From: Stephen T. <st...@sb...> - 2003-02-27 21:37:29
|
On Thu, 2003-02-27 at 15:26, Michael Roitzsch wrote: > Wanted to do this anyway. Do I have time until weekend? I can start if you want me to. > Btw, I don't think kicking out the assert()s in libdvdnav is necessary.=20 > I have watched a lot of DVDs and haven't seen one triggering for ages.=20 > But I won't keep anyone from doing it anyway. Patches can be sent to=20 > dvd-devel where I will receive them. (But don't call it XINE_ASSERT in=20 > libdvdnav ;) We are kicking out the asserts() from any file by "name" only. What I mean is that we are maintaining the same functionality which if a condition is true print something out then call abort().=20 This is the code that we installing.=20 /* Code Taken from GNU C Library manual */ /* Obtain a backtrace and print it to stdout. */ static inline void print_trace (void) { void *array[10]; size_t size; char **strings; size_t i; size =3D backtrace (array, 10); strings =3D backtrace_symbols (array, size); printf ("Obtained %zd stack frames.\n", size); for (i =3D 0; i < size; i++) printf ("%s\n", strings[i]); free (strings); } /** * Provide assert like feature with better description of failure=20 * Thanks to Mark Thomas=20 */=20 #define XINE_ASSERT(exp, desc, args...) \ if (!(exp)) { \ printf("%s:%s:%d: assertion `" #exp "' failed. " desc "\n\n", \ __FILE__, __FUNCTION__, __LINE__, ##args); \ print_trace(); \ abort(); \ } I hope this gives you a better idea of what we are doing. Its more of a assert on steriods. Stephen --=20 Stephen Torri <st...@sb...> |
From: Michael R. <mr...@us...> - 2003-02-27 23:07:18
|
Hi Stephen, > > Wanted to do this anyway. Do I have time until weekend? > > I can start if you want me to. Go ahead, if you want to. But be aware that our libdvdread has one major difference to the original one that we have to keep: Every buffer allocated internally in libdvdread must be aligned at a 2k boundary. Otherwise, raw device reading will not work. There is a patch for those differences in src/input/libdvdread/diff_against_cvs.patch, but it is likely that it will fail to apply for the new version. > We are kicking out the asserts() from any file by "name" only. What I > mean is that we are maintaining the same functionality which if a > condition is true print something out then call abort(). > > This is the code that we installing. > > /* Code Taken from GNU C Library manual */ > /* Obtain a backtrace and print it to stdout. */ > static inline void > print_trace (void) > { > void *array[10]; > size_t size; > char **strings; > size_t i; > > size = backtrace (array, 10); > strings = backtrace_symbols (array, size); > > printf ("Obtained %zd stack frames.\n", size); > > for (i = 0; i < size; i++) > printf ("%s\n", strings[i]); > > free (strings); > } > > /** > * Provide assert like feature with better description of failure > * Thanks to Mark Thomas > */ > #define XINE_ASSERT(exp, desc, args...) \ > if (!(exp)) { \ > printf("%s:%s:%d: assertion `" #exp "' failed. " desc "\n\n", \ > __FILE__, __FUNCTION__, __LINE__, ##args); \ > print_trace(); \ > abort(); \ > } > > I hope this gives you a better idea of what we are doing. Its more of > a assert on steriods. Could we agree on not inlining print_trace(void)? If the compiler really inlines this on every assert(), the code gets bigger. And the performance gain by inlining is useless, since it will only make xine abort faster. ;) Michael -- printk("; corrupted filesystem mounted read/write - your computer will explode within 20 seconds ... but you wanted it so!\n"); 2.4.3 linux/fs/hpfs/super.c |
From: Michael R. <mr...@us...> - 2003-02-28 15:55:23
|
replying to myself, job done Hi, we have libdvdread 0.9.4 in cvs now. Michael -- "In my opinion MS is a lot better at making money than it is at making good operating systems" -Linus Torvalds, August 1997 |