[Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Салям! 31-Дек-2004 12:46 [EMAIL PROTECTED] (Luchezar Georgiev) wrote to [EMAIL PROTECTED]: > +++ inthndlr.c31 Dec 2004 12:46:21 - 1.87.2.13 > @@ -752,7 +752,6 @@ >return_user(); > - break; > @@ -1025,7 +1027,6 @@ >return_user(); > - break; I think, for readability purposes (to make understanding by new developers easier) `break' should be remained as comment. Something like: /* return_user() never returns, so "break" not need */ --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
That's really bad practice. The reason that it's there is so if, by reason of a bug or hardware failure of any sort, return_user() does really return, you will have bug that will be a nightmare to find. For the savings of less than 10 bytes, it's not worth the risk. Pat Arkady V.Belousov wrote: Салям! 31-Дек-2004 12:46 [EMAIL PROTECTED] (Luchezar Georgiev) wrote to [EMAIL PROTECTED]: +++ inthndlr.c31 Dec 2004 12:46:21 - 1.87.2.13 @@ -752,7 +752,6 @@ return_user(); - break; @@ -1025,7 +1027,6 @@ return_user(); - break; I think, for readability purposes (to make understanding by new developers easier) `break' should be remained as comment. Something like: /* return_user() never returns, so "break" not need */ --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Hi! 5-Янв-2005 08:30 [EMAIL PROTECTED] (Pat Villani) wrote to freedos-kernel@lists.sourceforge.net: >>>+++ inthndlr.c31 Dec 2004 12:46:21 - 1.87.2.13 >>> return_user(); >>>- break; >> I think, for readability purposes (to make understanding by new >>developers easier) `break' should be remained as comment. Something like: >>/* return_user() never returns, so "break" not need */ PV> That's really bad practice. The reason that it's there is so if, by PV> reason of a bug or hardware failure of any sort, return_user() does PV> really return, you will have bug that will be a nightmare to find. No, I say not this (that `break' should be present): I say, that new kernel developer may not know (yet) that return_user() never returns, so [s]he may wonder, why there is no `break' and why this is not a bug. So, commenting this trick may ease the understanding of this code. PV> For the savings of less than 10 bytes, it's not worth the risk. BTW, `break' here really (may) save some bytes, because tails merging. --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Hi Arkady, The "-" in the diff says that the break is removed entirely. Did you actually mean this, given your reply? I think you do save 2 or 3 bytes per break, depending on the compiler. However, I can relate to you an amusing experience. At one time, I did some consulting for Bell Labs. A few years after I left, AT&T had a major network failure in the northeast US. A friend who was still with AT&T told me that the failure was the result of a hardware problem that took the code into an untested branch of code. It was a function call inside a case that should have never returned and had no break at the end. The code fell into the next case and the system fell like a set dominoes. I'm not maintaining the kernel, so just my $0.02 -- which is worth less in Euros ;-) Pat Arkady V.Belousov wrote: Hi! 5-Янв-2005 08:30 [EMAIL PROTECTED] (Pat Villani) wrote to freedos-kernel@lists.sourceforge.net: +++ inthndlr.c31 Dec 2004 12:46:21 - 1.87.2.13 return_user(); - break; I think, for readability purposes (to make understanding by new developers easier) `break' should be remained as comment. Something like: /* return_user() never returns, so "break" not need */ PV> That's really bad practice. The reason that it's there is so if, by PV> reason of a bug or hardware failure of any sort, return_user() does PV> really return, you will have bug that will be a nightmare to find. No, I say not this (that `break' should be present): I say, that new kernel developer may not know (yet) that return_user() never returns, so [s]he may wonder, why there is no `break' and why this is not a bug. So, commenting this trick may ease the understanding of this code. PV> For the savings of less than 10 bytes, it's not worth the risk. BTW, `break' here really (may) save some bytes, because tails merging. --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Hi! 5-Янв-2005 09:19 [EMAIL PROTECTED] (Pat Villani) wrote to freedos-kernel@lists.sourceforge.net: PV> The "-" in the diff says that the break is removed entirely. Did you PV> actually mean this, given your reply? Yes. Break is removed, but later some new developer may wonder, "why there are no `break'?". So, I suggest, better to remain `break' in place, or replace it by consequent comment (like there is commented other `break' missings - see "/* fall through */" comments). Of course, advanced developer later spends his time and teaches itself, that "return_user()" is never returns, but before this he will be in trouble. PV> I think you do save 2 or 3 bytes per break, depending on the compiler. `Break' is a jump, so removing `break' may reduce code, but if there are similar tails above removed `break's, then you (may) lost in size, because those tail now can't be joined (at least, if you can't instruct compiler, that last function call never returns). PV> However, I can relate to you an amusing experience. At one time, I did PV> some consulting for Bell Labs. A few years after I left, AT&T had a PV> major network failure in the northeast US. A friend who was still with PV> AT&T told me that the failure was the result of a hardware problem that PV> took the code into an untested branch of code. It was a function call PV> inside a case that should have never returned and had no break at the PV> end. The code fell into the next case and the system fell like a set PV> dominoes. :) We can't protect from such hardware failures (when executed random pieces of code). :( PV> I'm not maintaining the kernel, so just my $0.02 -- which is worth less PV> in Euros ;-) --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Arkady V.Belousov wrote: :) We can't protect from such hardware failures (when executed random pieces of code). :( Actually, you can. Changing something like this is the difference between a stable and unstable kernel. BTW -- that wasn't random code execution. It was an untested piece of code that didn't get executed because that hardware failure never occurred. How much of the kernel can any of us say has actually been executed? Are there any hardware error cases that are not caught in the driver or has just not been exercised because the error almost never happens? Pat --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
Re: [Freedos-kernel] Re: [Freedos-cvs] kernel/kernel inthndlr.c,1.87.2.12,1.87.2.13
Hi! 5-Янв-2005 12:07 [EMAIL PROTECTED] (Pat Villani) wrote to freedos-kernel@lists.sourceforge.net: >> :) We can't protect from such hardware failures (when executed random >>pieces of code). :( PV> Actually, you can. No - because hardware failures (on which code works) may be _very_ different and all of them is unpredictable. Hm. After rereading your (skipped) paragraph, I suggest that you mean "for handling external event was called code, which earlier wasn't tested". In this case, we discuss not "hardware failure", but "untested code". PV> Changing something like this is the difference PV> between a stable and unstable kernel. BTW -- that wasn't random code PV> execution. It was an untested piece of code that didn't get executed PV> because that hardware failure never occurred. But in given case (`break' after `return_user()') there are _no_ untested code, and return_user() doesn't returns back to switch in inthndlr.c unconditionally. So, `break' there is _really_ not need, but its missing (or missing comment about it) may mislead programmer and somewhat affect optimizer. PV> How much of the kernel PV> can any of us say has actually been executed? Are there any hardware PV> error cases that are not caught in the driver or has just not been PV> exercised because the error almost never happens? --- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almosthttp://www.thinkgeek.com/sfshirt ___ Freedos-kernel mailing list Freedos-kernel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-kernel
