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, ATT had a major network failure in the northeast US. A friend who was still with ATT 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, ATT had a PV major network failure in the northeast US. A friend who was still with PV ATT 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
[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
