Re: Is it really necessary for fatal() to dump core?
Patch at: http://www.squid-cache.org/bugs/show_bug.cgi?id=2673 On 19/05/2009, at 1:50 PM, Mark Nottingham wrote: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
R: Is it really necessary for fatal() to dump core?
-Messaggio originale- Da: adrian.ch...@gmail.com [mailto:adrian.ch...@gmail.com] Per conto di Adrian Chadd Inviato: martedì 19 maggio 2009 6.26 A: Mark Nottingham Cc: Squid Developers Oggetto: Re: Is it really necessary for fatal() to dump core? just make that behaviour configurable? core_on_fatal {on|off} +1 Regards Guido Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com Guido Serassio Acme Consulting S.r.l. - Microsoft Certified Partner Via Lucia Savarino, 110098 - Rivoli (TO) - ITALY Tel. : +39.011.9530135 Fax. : +39.011.9781115 Email: i...@acmeconsulting.it WWW: http://www.acmeconsulting.it/
Re: Is it really necessary for fatal() to dump core?
I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... On 19/05/2009, at 2:25 PM, Adrian Chadd wrote: just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Re: Is it really necessary for fatal() to dump core?
2009/5/19 Mark Nottingham m...@yahoo-inc.com: I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... Well, I'd be interested in seeing some better defined characteristics of stuff with some sort of defined expectations and behaviour. Like an API. :) Right now, fatal, assert, etc are all used interchangably for quite a wide variety of reasons and the codebase may be much better off if someone starts off by fixing these a bit. Adrian
Re: Is it really necessary for fatal() to dump core?
I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... That I agree with. Grep the code for all fatals and see what falls out. I think you will find it's only the config parser that can get away with non-core fatals. Amos On 19/05/2009, at 2:25 PM, Adrian Chadd wrote: just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Re: Is it really necessary for fatal() to dump core?
Yeah; looking through, it looks to me like a few in the auth and fs modules need to be changed to fatal_dump; the rest (over a hundred) don't really need a core -- they're things like config parsing errors and self-evident error states. I'll come up with a patch that does that and adds a FATAL_DUMPS compile-time flag with a 0 default. On 20/05/2009, at 11:28 AM, Mark Nottingham wrote: The case that triggered this for me was when the log daemon dies; doesn't make much sense to core there either. I'll take a look through and report back. On 20/05/2009, at 11:24 AM, Amos Jeffries wrote: I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... That I agree with. Grep the code for all fatals and see what falls out. I think you will find it's only the config parser that can get away with non-core fatals. Amos On 19/05/2009, at 2:25 PM, Adrian Chadd wrote: just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Re: Is it really necessary for fatal() to dump core?
Yeah; looking through, it looks to me like a few in the auth and fs modules need to be changed to fatal_dump; the rest (over a hundred) don't really need a core -- they're things like config parsing errors and self-evident error states. I'll come up with a patch that does that and adds a FATAL_DUMPS compile-time flag with a 0 default. What do you mean by that last? I think if you are going to correct the fatal() and fatal_dump() usage it won't be needed. Amos On 20/05/2009, at 11:28 AM, Mark Nottingham wrote: The case that triggered this for me was when the log daemon dies; doesn't make much sense to core there either. I'll take a look through and report back. On 20/05/2009, at 11:24 AM, Amos Jeffries wrote: I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... That I agree with. Grep the code for all fatals and see what falls out. I think you will find it's only the config parser that can get away with non-core fatals. Amos On 19/05/2009, at 2:25 PM, Adrian Chadd wrote: just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Re: Is it really necessary for fatal() to dump core?
Fine by me. On 20/05/2009, at 12:25 PM, Amos Jeffries wrote: Yeah; looking through, it looks to me like a few in the auth and fs modules need to be changed to fatal_dump; the rest (over a hundred) don't really need a core -- they're things like config parsing errors and self-evident error states. I'll come up with a patch that does that and adds a FATAL_DUMPS compile-time flag with a 0 default. What do you mean by that last? I think if you are going to correct the fatal() and fatal_dump() usage it won't be needed. Amos On 20/05/2009, at 11:28 AM, Mark Nottingham wrote: The case that triggered this for me was when the log daemon dies; doesn't make much sense to core there either. I'll take a look through and report back. On 20/05/2009, at 11:24 AM, Amos Jeffries wrote: I'm going to push back on that; the administrator doesn't really have any need to get a core when, for example, append_domain doesn't start with .'. Squid.conf is bloated as it is; if there are cases where a core could be conceivably useful, they should be converted to fatal_dump. From what I've seen they'll be a small minority at best... That I agree with. Grep the code for all fatals and see what falls out. I think you will find it's only the config parser that can get away with non-core fatals. Amos On 19/05/2009, at 2:25 PM, Adrian Chadd wrote: just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com -- Mark Nottingham m...@yahoo-inc.com
Is it really necessary for fatal() to dump core?
tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com
Re: Is it really necessary for fatal() to dump core?
just make that behaviour configurable? core_on_fatal {on|off} Adrian 2009/5/19 Mark Nottingham m...@yahoo-inc.com: tools.c:fatal() dumps core because it calls abort. Considering that the core can be quite large (esp. on a 64bit system), and that there's fatal_dump() as well if you really want one, can we just make fatal() exit(1) instead of abort()ing? Cheers, -- Mark Nottingham m...@yahoo-inc.com