Re: [PATCH 1/4] Implement 'set honor-dontdump-flag' command

[Pedro Alves <palves@redhat.com>]

On 11/28/2017 03:42 PM, Sergio Durigan Junior wrote:

>> @@ -2517,4 +2522,16 @@ of /proc/PID/coredump_filter when generating the corefile.  For more information
>>  about this file, refer to the manpage of core(5)."),
>>  			   NULL, show_use_coredump_filter,
>>  			   &setlist, &showlist);
>> +
>> +  add_setshow_boolean_cmd ("honor-dontdump-flag", class_files,
>> +			   &honor_dontdump_flag, _("\
>> +Set whether gcore should honor the VM_DONTDUMP flag."),
>> +			   _("\
>> +Show whether gcore should honor the VM_DONTDUMP flag."),
>> +			   _("\
>> +Use this command to set whether gcore should honor the VM_DONTDUMP\n\
>> +flag from /proc/PID/smaps when generating the corefile.  For more information\n\
>> +about this file, refer to the manpage of proc(5) and core(5)."),
>> +			   NULL, show_use_coredump_filter,
> 
> You've already spotted the mistake of using 'show_use_coredump_filter'
> here.
> 
>> +			   &setlist, &showlist);
> 
> I'm not sure "honor-dontdump-flag" is a good name for this setting.
> There's no indication that it relates to coredumps, and I think it
> should.  A name like "honor-coredump-dontdump-flag" is a bit repetitive,
> but IMHO is better than just "honor-dontdump-flag".  WDYT?

Personally, I don't find the "coredump" issue that much of a
problem, given "dump" is there.  "honor-coredump-dontdump" looks
like a mouthful to me.

I think using "ignore" in the name would some more natural
in GDB than "honor".  I.e., "set ignore-dontdump-flag on/off".
That mean default is off/0 ( and the control variable could go
to .bss :-) )

It also avoids our UK friends cursing at bad spelling of "honour".  :-)

"set dump-excluded-mappings on/off" could work too?

I skimmed the series and didn't find a gdb/NEWS entry; we need
one for new commands.

> 
> To be fair, my first thought was to suggest adding a new "set coredump"
> super command, which would encompass "set coredump use-coredump-filter"
> and "set coredump honor-dontdump-flag", but I wouldn't like to introduce
> this change so close to a release.
> 
> Otherwise, the patch looks OK to me, but I'm not a global maintainer.
> 

Thanks,
Pedro Alves