About core_dump_setup was Re: [SCM] Samba Shared Repository - branch master updated

[Matthieu Patou]

On 06/11/2012 01:44 AM, Andreas Schneider wrote:
> On Friday 08 June 2012 06:34:02 Matthieu Patou wrote:
>> The branch, master has been updated
>>         via  0ea7152 s3-winbindd: call dump_core_setup after command line
>> option has been parsed from  48b6c6a s3-waf: Fix the winbindd active
>> directory idmap support build.
>>
>> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
>>
>>
>> - Log -----------------------------------------------------------------
>> commit 0ea7152ff479e32e25817e4ddb1c2a1569bad81d
>> Author: Matthieu Patou<mat at matws.net>
>> Date:   Fri Jun 1 15:33:04 2012 -0700
>>
>>      s3-winbindd: call dump_core_setup after command line option has been
>> parsed
>>
>>      Without this fix in some situations winbindd can't coredump.
>>      Such cases append when samba is compiled in a custom prefix (ie.
>>      /home/build/mat/prod/1/) in this case get_dyn_LOGFILEBASE or
>> basename(lp_logfile) before the configuration file and the command line is
>> parsed will be something like /home/build/mat/prod/1/var which might not
>> exists on the host where you run it (where it's most probably more "normal"
>> directories).
>>      Specifying --log-basename didn't help as dump_core_setup is called
>> before the command line and the config file is read so it didn't help
>> getting a correct value in dump_core_setup. We fix this issue by calling
>> dump_core_setup() also after the command line has been read and also after
>> the configfile has been parsed so that the final location for the coredump
>> is coherent with the final logile location.
>>
>>      Autobuild-User(master): Matthieu Patou<mat at samba.org>
>>      Autobuild-Date(master): Fri Jun  8 06:33:33 CEST 2012 on sn-devel-104
>>
>> -----------------------------------------------------------------------
>>
>> Summary of changes:
>>   source3/winbindd/winbindd.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>>
>> Changeset truncated at 500 lines:
>>
>> diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
>> index 248f747..20ee2fc 100644
>> --- a/source3/winbindd/winbindd.c
>> +++ b/source3/winbindd/winbindd.c
>> @@ -1350,6 +1350,7 @@ int main(int argc, char **argv, char **envp)
>>   		}
>>   	}
>>
>> +	dump_core_setup("winbindd", lp_logfile());
>>   	if (is_daemon&&  interactive) {
>>   		d_fprintf(stderr,"\nERROR: "
>>   			  "Option -i|--interactive is not allowed together with
>> -D|--daemon\n\n"); @@ -1389,6 +1390,7 @@ int main(int argc, char **argv,
>> char **envp) DEBUG(0, ("error opening config file '%s'\n",
>> get_dyn_CONFIGFILE())); exit(1);
>>   	}
>> +	dump_core_setup("winbindd", lp_logfile());
>
> For such changes I would expect a comment in the source code that this needs
> to be called after option parsing so that someone who will refactor code later
> will not break it again!!!
I didn't think of it because I tend to have a look at git commit before 
removing code that seems to be useless, but that's my way of working. 
Thanks for pointing it I'll push a comment patch.

Matthieu.