Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch#2 3/6] set auto-load local-gdbinit warn-and-*
@ 2012-03-29  9:13 Jan Kratochvil
  2012-03-29 20:58 ` Doug Evans
  2012-03-30  7:26 ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: Jan Kratochvil @ 2012-03-29  9:13 UTC (permalink / raw)
  To: gdb-patches

Hi,

it does not change any default behavior (that's done in [patch 6/6]) but it
gives an option to warn on .gdbinit files (which become deprecated).


Thanks,
Jan


gdb/
2012-03-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* NEWS (set auto-load local-gdbinit): Add warn-and-on and warn-and-off
	parameters.
	* auto-load.c (auto_load_local_gdbinit_yes, auto_load_local_gdbinit_no)
	(auto_load_local_gdbinit_warn_and_yes)
	(auto_load_local_gdbinit_warn_and_no, auto_load_local_gdbinit_enum):
	New.
	(auto_load_local_gdbinit): Change it to string.
	(_initialize_auto_load): Extend the parameters
	for "set auto-load local-gdbinit".
	* auto-load.h (auto_load_local_gdbinit_yes, auto_load_local_gdbinit_no)
	(auto_load_local_gdbinit_warn_and_yes) 
	(auto_load_local_gdbinit_warn_and_no): New prototypes.
	(auto_load_local_gdbinit): Change the prototype to string.
	* main.c: Include readline/tilde.h.
	(get_init_files): Add parameter local_gdbinit_stat, describe it in the
	comment.  New variable localinit_stat.  Remove variable cwdbuf.
	(captured_main): New variable local_gdbinit_stat.  Scan CMDARG_VEC and
	print warnings according to AUTO_LOAD_LOCAL_GDBINIT.
	(print_gdb_help): Update the get_init_files caller.

gdb/doc/
2012-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Startup): Mention also warn-and-yes for local gdbinit,
	deprecate the feature.
	(Auto-loading): Update the example from on to yes.
	(Current Directory Init File): Mention also warn-and-yes and
	warn-and-no options of "set auto-load local-gdbinit and the warnings for
	"show auto-load local-gdbinit".

--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -141,7 +141,7 @@ set auto-load python-scripts on|off
 show auto-load python-scripts
   Control auto-loading of Python script files.
 
-set auto-load local-gdbinit on|off
+set auto-load local-gdbinit on|off|warn-and-on|warn-and-off
 show auto-load local-gdbinit
   Control loading of init file (.gdbinit) from current directory.
 
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -70,8 +70,31 @@ show_auto_load_gdb_scripts (struct ui_file *file, int from_tty,
    used to find the scripts.  */
 int global_auto_load = 1;
 
+/* Load current directory .gdbinit file automatically.  */
+const char auto_load_local_gdbinit_yes[] = "yes";
+
+/* Never load current directory .gdbinit file.  */
+const char auto_load_local_gdbinit_no[] = "no";
+
+/* Give deprecation warning and load current directory .gdbinit file
+   automatically.  */
+const char auto_load_local_gdbinit_warn_and_yes[] = "warn-and-yes";
+
+/* Give deprecation warning and do not load current directory .gdbinit file.  */
+const char auto_load_local_gdbinit_warn_and_no[] = "warn-and-no";
+
+/* Options for auto_load_local_gdbinit.  */
+static const char *const auto_load_local_gdbinit_enum[] =
+{
+  auto_load_local_gdbinit_yes,
+  auto_load_local_gdbinit_no,
+  auto_load_local_gdbinit_warn_and_yes,
+  auto_load_local_gdbinit_warn_and_no,
+  NULL
+};
+
 /* Auto-load .gdbinit file from the current directory?  */
-int auto_load_local_gdbinit = 1;
+const char *auto_load_local_gdbinit = auto_load_local_gdbinit_yes;
 
 /* Absolute pathname to the current directory .gdbinit, if it exists.  */
 char *auto_load_local_gdbinit_pathname = NULL;
@@ -705,18 +728,20 @@ This options has security implications for untrusted inferiors."),
 Usage: info auto-load gdb-scripts [REGEXP]"),
 	   auto_load_info_cmdlist_get ());
 
-  add_setshow_boolean_cmd ("local-gdbinit", class_support,
-			   &auto_load_local_gdbinit, _("\
+  add_setshow_enum_cmd ("local-gdbinit", class_support,
+			auto_load_local_gdbinit_enum,
+			&auto_load_local_gdbinit, _("\
 Enable or disable auto-loading of .gdbinit script in current directory."), _("\
 Show whether auto-loading .gdbinit script in current directory is enabled."),
 			   _("\
 If enabled, canned sequences of commands are loaded when debugger starts\n\
 from .gdbinit file in current directory.  Such files are deprecated,\n\
 use script associated with inferior executable file instead.\n\
+You can optionally display a warning when such file is found.\n\
 This options has security implications for untrusted inferiors."),
-			   NULL, show_auto_load_local_gdbinit,
-			   auto_load_set_cmdlist_get (),
-			   auto_load_show_cmdlist_get ());
+			NULL, show_auto_load_local_gdbinit,
+			auto_load_set_cmdlist_get (),
+			auto_load_show_cmdlist_get ());
 
   add_cmd ("local-gdbinit", class_info, info_auto_load_local_gdbinit,
 	   _("Print whether current directory .gdbinit file has been loaded.\n\
--- a/gdb/auto-load.h
+++ b/gdb/auto-load.h
@@ -32,7 +32,11 @@ struct script_language
 
 extern int global_auto_load;
 
-extern int auto_load_local_gdbinit;
+extern const char auto_load_local_gdbinit_yes[];
+extern const char auto_load_local_gdbinit_no[];
+extern const char auto_load_local_gdbinit_warn_and_yes[];
+extern const char auto_load_local_gdbinit_warn_and_no[];
+extern const char *auto_load_local_gdbinit;
 extern char *auto_load_local_gdbinit_pathname;
 extern int auto_load_local_gdbinit_loaded;
 
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1287,13 +1287,16 @@ Processes command line options and operands.
 @item
 Reads and executes the commands from init file (if any) in the current
 working directory as long as @samp{set auto-load local-gdbinit} is set to
-@samp{yes} (@pxref{Current Directory Init File}).
+@samp{yes} or @samp{warn-and-yes} (@pxref{Current Directory Init File}).
 This is only done if the current directory is
 different from your home directory.  Thus, you can have more than one
 init file, one generic in your home directory, and another, specific
 to the program you are debugging, in the directory where you invoke
 @value{GDBN}.
 
+This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;
+it will be file-bound, no longer directory-bound.
+
 @item
 If the command line specified a program to debug, or a process to
 attach to, or a core file, @value{GDBN} loads any auto-loaded
@@ -20728,7 +20731,7 @@ is enabled or disabled.
 (gdb) show auto-load
 gdb-scripts:  Canned sequences of commands auto-loading is on.
 libthread-db:  Inferior specific libthread_db auto-loading is on.
-local-gdbinit:  Current directory .gdbinit script auto-loading is on.
+local-gdbinit:  Current directory .gdbinit script auto-loading is yes.
 python-scripts:  Python scripts auto-loading is on.
 @end smallexample
 
@@ -20816,15 +20819,20 @@ see @ref{Current Directory Init File during Startup}.
 @table @code
 @anchor{set auto-load local-gdbinit}
 @kindex set auto-load local-gdbinit
-@item set auto-load local-gdbinit [yes|no]
+@item set auto-load local-gdbinit [yes|no|warn-and-yes|warn-and-no]
 Enable or disable the auto-loading of canned sequences of commands
 (@pxref{Sequences}) found in init file in the current directory.
+The options @samp{warn-and-X} give warning before loading
+(@samp{warn-and-yes}) or when declining to load (@samp{warn-and-no}) the file
+as the @samp{local-gdbinit} feature is deprecated in the favor
+of @xref{objfile-gdb.rc file}.
 
 @anchor{show auto-load local-gdbinit}
 @kindex show auto-load local-gdbinit
 @item show auto-load local-gdbinit
 Show whether auto-loading of canned sequences of commands from init file in the
-current directory is enabled or disabled.
+current directory is enabled or disabled and whether warnings are printed
+during its load.
 
 @anchor{info auto-load local-gdbinit}
 @kindex info auto-load local-gdbinit
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -42,6 +42,7 @@
 #include "python/python.h"
 #include "objfiles.h"
 #include "auto-load.h"
+#include "readline/tilde.h"
 
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
@@ -146,23 +147,26 @@ relocate_gdb_directory (const char *initial, int flag)
 }
 
 /* Compute the locations of init files that GDB should source and
-   return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
-   there is no system gdbinit (resp. home gdbinit and local gdbinit)
-   to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
-   LOCAL_GDBINIT) is set to NULL.  */
+   return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT and
+   LOCAL_GDBINIT_STAT.  If there is no system gdbinit (resp. home
+   gdbinit and local gdbinit) to be loaded, then SYSTEM_GDBINIT (resp.
+   HOME_GDBINIT and LOCAL_GDBINIT) is set to NULL, LOCAL_GDBINIT_STAT is
+   zeroed.  */
+
 static void
 get_init_files (char **system_gdbinit,
 		char **home_gdbinit,
-		char **local_gdbinit)
+		char **local_gdbinit, struct stat *local_gdbinit_stat)
 {
   static char *sysgdbinit = NULL;
   static char *homeinit = NULL;
   static char *localinit = NULL;
+  static struct stat localinit_stat;
   static int initialized = 0;
 
   if (!initialized)
     {
-      struct stat homebuf, cwdbuf, s;
+      struct stat homebuf, s;
       char *homedir, *relocated_sysgdbinit;
 
       if (SYSTEM_GDBINIT[0])
@@ -180,12 +184,12 @@ get_init_files (char **system_gdbinit,
 
       /* If the .gdbinit file in the current directory is the same as
 	 the $HOME/.gdbinit file, it should not be sourced.  homebuf
-	 and cwdbuf are used in that purpose.  Make sure that the stats
-	 are zero in case one of them fails (this guarantees that they
-	 won't match if either exists).  */
+	 and localinit_stat are used in that purpose.  Make sure
+	 that the stats are zero in case one of them fails (this
+	 guarantees that they won't match if either exists).  */
 
       memset (&homebuf, 0, sizeof (struct stat));
-      memset (&cwdbuf, 0, sizeof (struct stat));
+      memset (&localinit_stat, 0, sizeof (struct stat));
 
       if (homedir)
 	{
@@ -197,11 +201,10 @@ get_init_files (char **system_gdbinit,
 	    }
 	}
 
-      if (stat (gdbinit, &cwdbuf) == 0)
+      if (stat (gdbinit, &localinit_stat) == 0)
 	{
 	  if (!homeinit
-	      || memcmp ((char *) &homebuf, (char *) &cwdbuf,
-			 sizeof (struct stat)))
+	      || memcmp (&homebuf, &localinit_stat, sizeof (struct stat)))
 	    localinit = gdbinit;
 	}
       
@@ -211,6 +214,8 @@ get_init_files (char **system_gdbinit,
   *system_gdbinit = sysgdbinit;
   *home_gdbinit = homeinit;
   *local_gdbinit = localinit;
+  if (local_gdbinit_stat)
+    *local_gdbinit_stat = localinit_stat;
 }
 
 /* Call command_loop.  If it happens to return, pass that through as a
@@ -303,6 +308,7 @@ captured_main (void *data)
   char *system_gdbinit;
   char *home_gdbinit;
   char *local_gdbinit;
+  struct stat local_gdbinit_stat;
 
   int i;
   int save_auto_load;
@@ -755,7 +761,8 @@ captured_main (void *data)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
-  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
+  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit,
+		  &local_gdbinit_stat);
 
   /* Do these (and anything which might call wrap_here or *_filtered)
      after initialize_all_files() but before the interpreter has been
@@ -944,12 +951,64 @@ captured_main (void *data)
     {
       auto_load_local_gdbinit_pathname = gdb_realpath (local_gdbinit);
 
-      if (!inhibit_gdbinit && auto_load_local_gdbinit)
+      if (!inhibit_gdbinit
+	  && auto_load_local_gdbinit != auto_load_local_gdbinit_no)
 	{
-	  auto_load_local_gdbinit_loaded = 1;
+	  /* Verify whether user has already specified `-x ./.gdbinit'.  */
+	  for (i = 0; VEC_iterate (cmdarg_s, cmdarg_vec, i, cmdarg_p); i++)
+	    if (cmdarg_p->type == CMDARG_FILE)
+	      {
+		struct cleanup *old_cleanups;
+		char *file;
+		int fd;
+
+		file = tilde_expand (cmdarg_p->string);
+		old_cleanups = make_cleanup (xfree, file);
+
+		fd = openp (source_path, OPF_TRY_CWD_FIRST, file, O_RDONLY,
+			    NULL);
+
+		do_cleanups (old_cleanups);
+
+		if (fd != -1)
+		  {
+		    struct stat statbuf;
+
+		    if (fstat (fd, &statbuf) == 0
+			&& memcmp (&statbuf, &local_gdbinit_stat,
+				   sizeof (statbuf)) == 0)
+		      local_gdbinit = NULL;
 
-	  catch_command_errors (source_script, local_gdbinit, 0,
-				RETURN_MASK_ALL);
+		    close (fd);
+		    if (local_gdbinit == NULL)
+		      break;
+		  }
+	      }
+
+	  if (local_gdbinit)
+	    {
+	      const char adv[] = N_("Use script associated with inferior "
+				    "executable file instead.  "
+				    "See also 'set auto-load local-gdbinit'.  "
+				    "You may also use 'gdb -x .gdbinit ...'.");
+	      if (auto_load_local_gdbinit
+		  == auto_load_local_gdbinit_warn_and_yes)
+		warning (_("Reading file .gdbinit in current directory but it "
+			   "has been deprecated and the reading will be "
+			   "removed.  %s"),
+			 _(adv));
+	      if (auto_load_local_gdbinit
+		  == auto_load_local_gdbinit_warn_and_no)
+		warning (_("Ignoring file .gdbinit in current directory as it "
+			   "has been deprecated.  %s"),
+			 _(adv));
+	      else
+		{
+		  auto_load_local_gdbinit_loaded = 1;
+		  catch_command_errors (source_script, local_gdbinit, 0,
+					RETURN_MASK_ALL);
+		}
+	    }
 	}
     }
 
@@ -1021,7 +1080,7 @@ print_gdb_help (struct ui_file *stream)
   char *home_gdbinit;
   char *local_gdbinit;
 
-  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
+  get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit, NULL);
 
   fputs_unfiltered (_("\
 This is the GNU debugger.  Usage:\n\n\


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-29  9:13 [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
@ 2012-03-29 20:58 ` Doug Evans
  2012-03-30  6:28   ` Eli Zaretskii
  2012-03-30  7:26 ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: Doug Evans @ 2012-03-29 20:58 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Thu, Mar 29, 2012 at 2:12 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> it does not change any default behavior (that's done in [patch 6/6]) but it
> gives an option to warn on .gdbinit files (which become deprecated).

Deprecating .gdbinit files requires a NEWS entry.
[But maybe that's in a later patch.]

And are we *really* sure we want to deprecate them?
I don't read every thread (and forget half I do read :-() so I may
have missed a high level decision.


> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -141,7 +141,7 @@ set auto-load python-scripts on|off
>  show auto-load python-scripts
>   Control auto-loading of Python script files.
>
> -set auto-load local-gdbinit on|off
> +set auto-load local-gdbinit on|off|warn-and-on|warn-and-off
>  show auto-load local-gdbinit
>   Control loading of init file (.gdbinit) from current directory.
>

While I realize you might be trying to avoid adding another option,
combining warn on/off with auto-load on/off is a bit odd.  Is it
possible to look at this differently and have the warning apply to
more than just local-gdbinit (thus being a more useful option and thus
being less of a pain to add)?
E.g., a warning for using deprecated features or some such.
[Or maybe we want a warning for each such feature, dunno.]

> +                   struct stat statbuf;
> +
> +                   if (fstat (fd, &statbuf) == 0
> +                       && memcmp (&statbuf, &local_gdbinit_stat,
> +                                  sizeof (statbuf)) == 0)
> +                     local_gdbinit = NULL;

This assumes absence of padding.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-29 20:58 ` Doug Evans
@ 2012-03-30  6:28   ` Eli Zaretskii
  2012-03-30  6:33     ` Doug Evans
  2012-03-30 18:08     ` suspend: " Jan Kratochvil
  0 siblings, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2012-03-30  6:28 UTC (permalink / raw)
  To: Doug Evans; +Cc: jan.kratochvil, gdb-patches

> Date: Thu, 29 Mar 2012 13:57:50 -0700
> From: Doug Evans <dje@google.com>
> Cc: gdb-patches@sourceware.org
> 
> Deprecating .gdbinit files requires a NEWS entry.
> [But maybe that's in a later patch.]
> 
> And are we *really* sure we want to deprecate them?

Looks like we are.  FWIW, I would suggest polling at least some of the
projects that provide .gdbinit as part of their distribution.  Even if
there's only one such project (Emacs), its developers should be
polled, IMO, before making this move.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-30  6:28   ` Eli Zaretskii
@ 2012-03-30  6:33     ` Doug Evans
  2012-03-30  6:45       ` Jan Kratochvil
  2012-03-30 18:08     ` suspend: " Jan Kratochvil
  1 sibling, 1 reply; 17+ messages in thread
From: Doug Evans @ 2012-03-30  6:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jan.kratochvil, gdb-patches

On Thu, Mar 29, 2012 at 11:26 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 29 Mar 2012 13:57:50 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: gdb-patches@sourceware.org
>>
>> Deprecating .gdbinit files requires a NEWS entry.
>> [But maybe that's in a later patch.]
>>
>> And are we *really* sure we want to deprecate them?
>
> Looks like we are.  FWIW, I would suggest polling at least some of the
> projects that provide .gdbinit as part of their distribution.  Even if
> there's only one such project (Emacs), its developers should be
> polled, IMO, before making this move.

I'd reword that to say that before we hear from some significant
representative users we should not be sure we're ready.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-30  6:33     ` Doug Evans
@ 2012-03-30  6:45       ` Jan Kratochvil
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Kratochvil @ 2012-03-30  6:45 UTC (permalink / raw)
  To: Doug Evans; +Cc: Eli Zaretskii, gdb-patches

On Fri, 30 Mar 2012 08:33:29 +0200, Doug Evans wrote:
> On Thu, Mar 29, 2012 at 11:26 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> And are we *really* sure we want to deprecate them?
> >
> > Looks like we are.  FWIW, I would suggest polling at least some of the
> > projects that provide .gdbinit as part of their distribution.  Even if
> > there's only one such project (Emacs), its developers should be
> > polled, IMO, before making this move.
> 
> I'd reword that to say that before we hear from some significant
> representative users we should not be sure we're ready.

OK; to restate that .gdbinit loading is not a security part of this patchset.
"set autoload safepath" is.
I also made a mistake stating I will change the default in downstream
(Fedora), it does not need to be done, "safepath" is the part fixing the
security hole, "local-gdbinit" is unrelated.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-29  9:13 [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
  2012-03-29 20:58 ` Doug Evans
@ 2012-03-30  7:26 ` Eli Zaretskii
  2012-04-02 20:31   ` [doc patch] objfile -> @var{objfile} [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*] Jan Kratochvil
  2012-04-03 18:33   ` [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
  1 sibling, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2012-03-30  7:26 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> Date: Thu, 29 Mar 2012 11:12:58 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> it does not change any default behavior (that's done in [patch 6/6]) but it
> gives an option to warn on .gdbinit files (which become deprecated).

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -141,7 +141,7 @@ set auto-load python-scripts on|off
>  show auto-load python-scripts
>    Control auto-loading of Python script files.
>  
> -set auto-load local-gdbinit on|off
> +set auto-load local-gdbinit on|off|warn-and-on|warn-and-off
>  show auto-load local-gdbinit
>    Control loading of init file (.gdbinit) from current directory.

OK.

>  If enabled, canned sequences of commands are loaded when debugger starts\n\
>  from .gdbinit file in current directory.  Such files are deprecated,\n\
>  use script associated with inferior executable file instead.\n\
   ^^^^^^^^^^
Either "use a script" or "use scripts", probably the former.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1287,13 +1287,16 @@ Processes command line options and operands.
>  @item
>  Reads and executes the commands from init file (if any) in the current
>  working directory as long as @samp{set auto-load local-gdbinit} is set to
> -@samp{yes} (@pxref{Current Directory Init File}).
> +@samp{yes} or @samp{warn-and-yes} (@pxref{Current Directory Init File}).
>  This is only done if the current directory is
>  different from your home directory.  Thus, you can have more than one
>  init file, one generic in your home directory, and another, specific
>  to the program you are debugging, in the directory where you invoke
>  @value{GDBN}.
>  
> +This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;

"objfile" should be in @var, and it should explain what is "objfile".

>  gdb-scripts:  Canned sequences of commands auto-loading is on.
>  libthread-db:  Inferior specific libthread_db auto-loading is on.
> -local-gdbinit:  Current directory .gdbinit script auto-loading is on.
> +local-gdbinit:  Current directory .gdbinit script auto-loading is yes.

Why "yes"?

> +as the @samp{local-gdbinit} feature is deprecated in the favor

"in favor", without "the".

> +of @xref{objfile-gdb.rc file}.

Again, "objfile" should be in @var.

OK with those changes.

> +		warning (_("Reading file .gdbinit in current directory but it "
> +			   "has been deprecated and the reading will be "
> +			   "removed.  %s"),

"reading will be removed" is very ambiguous.  Suggest something like

  Reading of .gdbinit files in current directory has been deprecated
  and will be removed in a future version of GDB.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* suspend: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-30  6:28   ` Eli Zaretskii
  2012-03-30  6:33     ` Doug Evans
@ 2012-03-30 18:08     ` Jan Kratochvil
  1 sibling, 0 replies; 17+ messages in thread
From: Jan Kratochvil @ 2012-03-30 18:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Doug Evans, gdb-patches

On Fri, 30 Mar 2012 08:26:45 +0200, Eli Zaretskii wrote:
> > From: Doug Evans <dje@google.com>
> > And are we *really* sure we want to deprecate them?
> 
> Looks like we are.  FWIW, I would suggest polling at least some of the
> projects that provide .gdbinit as part of their distribution.  Even if
> there's only one such project (Emacs), its developers should be
> polled, IMO, before making this move.

I will suspend this patch before the other parts get in, together with its
dependent:
	[patch#2 6/6] Warn (but execute) local .gdbinit by default

My primary concern now is the security and this is not related to security.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [doc patch] objfile -> @var{objfile}  [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*]
  2012-03-30  7:26 ` Eli Zaretskii
@ 2012-04-02 20:31   ` Jan Kratochvil
  2012-04-02 21:10     ` Eli Zaretskii
  2012-04-03 18:33   ` [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
  1 sibling, 1 reply; 17+ messages in thread
From: Jan Kratochvil @ 2012-04-02 20:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Fri, 30 Mar 2012 09:25:41 +0200, Eli Zaretskii wrote:
> > +This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;
> 
> "objfile" should be in @var, and it should explain what is "objfile".

But this means existing manual also needs this fix.

info works fine with that leading @var.


Thanks,
Jan


gdb/doc/
2012-04-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Auto-loading): Update two references.
	(objfile-gdb.py file): Rename to ...
	(@var{objfile}-gdb.py file): ... here.

--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24714,7 +24714,7 @@ writable.
 When a new object file is read (for example, due to the @code{file}
 command, or because the inferior has loaded a shared library),
 @value{GDBN} will look for Python support scripts in several ways:
-@file{@var{objfile}-gdb.py} (@pxref{objfile-gdb.py file})
+@file{@var{objfile}-gdb.py} (@pxref{@var{objfile}-gdb.py file})
 and @code{.debug_gdb_scripts} section
 (@pxref{dotdebug_gdb_scripts section}).
 
@@ -24764,12 +24764,12 @@ function (@pxref{Objfiles In Python}).  This can be useful for
 registering objfile-specific pretty-printers.
 
 @menu
-* objfile-gdb.py file::          The @file{@var{objfile}-gdb.py} file
+* @var{objfile}-gdb.py file::          The @file{@var{objfile}-gdb.py} file
 * dotdebug_gdb_scripts section:: The @code{.debug_gdb_scripts} section
 * Which flavor to choose?::
 @end menu
 
-@node objfile-gdb.py file
+@node @var{objfile}-gdb.py file
 @subsubsection The @file{@var{objfile}-gdb.py} file
 @cindex @file{@var{objfile}-gdb.py}
 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [doc patch] objfile -> @var{objfile}  [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*]
  2012-04-02 20:31   ` [doc patch] objfile -> @var{objfile} [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*] Jan Kratochvil
@ 2012-04-02 21:10     ` Eli Zaretskii
  2012-04-02 21:13       ` Jan Kratochvil
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2012-04-02 21:10 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> Date: Mon, 2 Apr 2012 22:30:55 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> On Fri, 30 Mar 2012 09:25:41 +0200, Eli Zaretskii wrote:
> > > +This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;
> > 
> > "objfile" should be in @var, and it should explain what is "objfile".
> 
> But this means existing manual also needs this fix.

Oops, sorry.  I don't know what I was smoking.  Of course,
cross-references and nodes do not use @var.  Sorry, sorry.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [doc patch] objfile -> @var{objfile}  [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*]
  2012-04-02 21:10     ` Eli Zaretskii
@ 2012-04-02 21:13       ` Jan Kratochvil
  2012-04-02 21:19         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Kratochvil @ 2012-04-02 21:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Mon, 02 Apr 2012 23:10:24 +0200, Eli Zaretskii wrote:
> > But this means existing manual also needs this fix.
> 
> Oops, sorry.  I don't know what I was smoking.  Of course,
> cross-references and nodes do not use @var.  Sorry, sorry.

But in practice it works OK and the formatting is then unified in all cases,
are you sure with this decision?


Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [doc patch] objfile -> @var{objfile}  [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*]
  2012-04-02 21:13       ` Jan Kratochvil
@ 2012-04-02 21:19         ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2012-04-02 21:19 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> Date: Mon, 2 Apr 2012 23:12:42 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> On Mon, 02 Apr 2012 23:10:24 +0200, Eli Zaretskii wrote:
> > > But this means existing manual also needs this fix.
> > 
> > Oops, sorry.  I don't know what I was smoking.  Of course,
> > cross-references and nodes do not use @var.  Sorry, sorry.
> 
> But in practice it works OK and the formatting is then unified in all cases,
> are you sure with this decision?

Yes, I'm sure.  The name of the node might be unfortunate, but using
@-commands in node names is discouraged, see the Texinfo manual.

I'm very sorry for my stupid mistake.  Somehow, I managed to miss the
fact that this was inside a @ref.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-03-30  7:26 ` Eli Zaretskii
  2012-04-02 20:31   ` [doc patch] objfile -> @var{objfile} [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*] Jan Kratochvil
@ 2012-04-03 18:33   ` Jan Kratochvil
  2012-04-05 16:15     ` Eli Zaretskii
  2012-04-05 16:28     ` Doug Evans
  1 sibling, 2 replies; 17+ messages in thread
From: Jan Kratochvil @ 2012-04-03 18:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Fri, 30 Mar 2012 09:25:41 +0200, Eli Zaretskii wrote:
> >  If enabled, canned sequences of commands are loaded when debugger starts\n\
> >  from .gdbinit file in current directory.  Such files are deprecated,\n\
> >  use script associated with inferior executable file instead.\n\
>    ^^^^^^^^^^
> Either "use a script" or "use scripts", probably the former.

Use "use a script" (it is in [patch 2/X]).


> > +This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;
> 
> "objfile" should be in @var, and it should explain what is "objfile".

Added:

This feature is deprecated, please use @ref{@var{objfile}-gdb.gdb file}
instead.  The canned sequence of commands will be then specific for loaded
@var{objfile}---typically the main executable---after converting an init file in
the current directory into the @var{objfile}-gdb.gdb file..


> >  gdb-scripts:  Canned sequences of commands auto-loading is on.
> >  libthread-db:  Inferior specific libthread_db auto-loading is on.
> > -local-gdbinit:  Current directory .gdbinit script auto-loading is on.
> > +local-gdbinit:  Current directory .gdbinit script auto-loading is yes.
> 
> Why "yes"?

parse_binary_operation accepts any of 'on', '1', 'yes' or 'enable' (and sure
off/0/no/disable).  It then displays the boolean only as "on" and "off".

Somehow it seems more appropriate for these auto-loading settings to use
yes/no instead of on/off.  This is also why I use yes/no in the documentation.
When I added the warning option I had to choose only one pair, therefore
I chose yes/no.  But this means the default displayed boolean values are new
yes/no (and not on/off).  It also means it no longer accepts the alternative
words on/1/enable (and off/0/disable); but that is not a real regression when
the setting never existed before this patchset.

There are two possibilities:
 * Introduce new option some "set local-gdbinit-warning <boolean>" as
   suggested by Doug IIUC and technically keep everything boolean.
 * Switch the words used in documentation from yes/no to on/off, therefore
   also make the switch on/off/warn-and-on/warn-and-off.



Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-04-03 18:33   ` [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
@ 2012-04-05 16:15     ` Eli Zaretskii
  2012-04-05 21:00       ` Jan Kratochvil
  2012-04-05 16:28     ` Doug Evans
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2012-04-05 16:15 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> Date: Tue, 3 Apr 2012 20:33:11 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> > > +This feature is deprecated, please use @ref{objfile-gdb.rc file} instead;
> > 
> > "objfile" should be in @var, and it should explain what is "objfile".
> 
> Added:
> 
> This feature is deprecated, please use @ref{@var{objfile}-gdb.gdb file}
> instead.  The canned sequence of commands will be then specific for loaded
> @var{objfile}---typically the main executable---after converting an init file in
> the current directory into the @var{objfile}-gdb.gdb file..

Sorry, the @var inside @ref is wrong, please revert that part.

> > >  gdb-scripts:  Canned sequences of commands auto-loading is on.
> > >  libthread-db:  Inferior specific libthread_db auto-loading is on.
> > > -local-gdbinit:  Current directory .gdbinit script auto-loading is on.
> > > +local-gdbinit:  Current directory .gdbinit script auto-loading is yes.
> > 
> > Why "yes"?
> 
> parse_binary_operation accepts any of 'on', '1', 'yes' or 'enable' (and sure
> off/0/no/disable).  It then displays the boolean only as "on" and "off".

But this hunk changes "on" into "yes", which is not what you seem to
say will happen.  Did I understand correctly that GDB will print "on"?

> Somehow it seems more appropriate for these auto-loading settings to use
> yes/no instead of on/off.  This is also why I use yes/no in the documentation.
> When I added the warning option I had to choose only one pair, therefore
> I chose yes/no.  But this means the default displayed boolean values are new
> yes/no (and not on/off).  It also means it no longer accepts the alternative
> words on/1/enable (and off/0/disable); but that is not a real regression when
> the setting never existed before this patchset.
> 
> There are two possibilities:
>  * Introduce new option some "set local-gdbinit-warning <boolean>" as
>    suggested by Doug IIUC and technically keep everything boolean.
>  * Switch the words used in documentation from yes/no to on/off, therefore
>    also make the switch on/off/warn-and-on/warn-and-off.

Whatever we do, I think these options should behave consistently.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-04-03 18:33   ` [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
  2012-04-05 16:15     ` Eli Zaretskii
@ 2012-04-05 16:28     ` Doug Evans
  2012-04-05 16:53       ` Jan Kratochvil
  1 sibling, 1 reply; 17+ messages in thread
From: Doug Evans @ 2012-04-05 16:28 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Eli Zaretskii, gdb-patches

On Tue, Apr 3, 2012 at 11:33 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
>> >  gdb-scripts:  Canned sequences of commands auto-loading is on.
>> >  libthread-db:  Inferior specific libthread_db auto-loading is on.
>> > -local-gdbinit:  Current directory .gdbinit script auto-loading is on.
>> > +local-gdbinit:  Current directory .gdbinit script auto-loading is yes.
>>
>> Why "yes"?
>
> parse_binary_operation accepts any of 'on', '1', 'yes' or 'enable' (and sure
> off/0/no/disable).  It then displays the boolean only as "on" and "off".
>
> Somehow it seems more appropriate for these auto-loading settings to use
> yes/no instead of on/off.  This is also why I use yes/no in the documentation.
> When I added the warning option I had to choose only one pair, therefore
> I chose yes/no.  But this means the default displayed boolean values are new
> yes/no (and not on/off).  It also means it no longer accepts the alternative
> words on/1/enable (and off/0/disable); but that is not a real regression when
> the setting never existed before this patchset.

"on" reads much better to me.

> There are two possibilities:
>  * Introduce new option some "set local-gdbinit-warning <boolean>" as
>   suggested by Doug IIUC and technically keep everything boolean.

For completeness sake,
I was suggesting making the option more general, applying to more to
more than just local-gdbinit (assuming more applications were readily
available).

>  * Switch the words used in documentation from yes/no to on/off, therefore
>   also make the switch on/off/warn-and-on/warn-and-off.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-04-05 16:28     ` Doug Evans
@ 2012-04-05 16:53       ` Jan Kratochvil
  2012-04-05 17:03         ` Doug Evans
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Kratochvil @ 2012-04-05 16:53 UTC (permalink / raw)
  To: Doug Evans; +Cc: Eli Zaretskii, gdb-patches

On Thu, 05 Apr 2012 18:27:51 +0200, Doug Evans wrote:
> "on" reads much better to me.

"on" sure reads OK but do you read OK "warn-and-on" and "warn-and-off"?


> > There are two possibilities:
> >  * Introduce new option some "set local-gdbinit-warning <boolean>" as
> >   suggested by Doug IIUC and technically keep everything boolean.
> 
> For completeness sake,
> I was suggesting making the option more general, applying to more to
> more than just local-gdbinit (assuming more applications were readily
> available).

The warning is there duue to local-gdbinit (possible) deprecation.  No other
file is being deprecated.

To apply it for example to use of deprecated GDB commands (and not files)
I somehow find too unrelated, I do not see any need / usefulness of such
warning.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-04-05 16:53       ` Jan Kratochvil
@ 2012-04-05 17:03         ` Doug Evans
  0 siblings, 0 replies; 17+ messages in thread
From: Doug Evans @ 2012-04-05 17:03 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Eli Zaretskii, gdb-patches

On Thu, Apr 5, 2012 at 9:53 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Thu, 05 Apr 2012 18:27:51 +0200, Doug Evans wrote:
>> "on" reads much better to me.
>
> "on" sure reads OK but do you read OK "warn-and-on" and "warn-and-off"?

on-and-warn?

At any rate, I wouldn't change it to "yes" just to change warn-and-on
to warn-and-yes.

>> > There are two possibilities:
>> >  * Introduce new option some "set local-gdbinit-warning <boolean>" as
>> >   suggested by Doug IIUC and technically keep everything boolean.
>>
>> For completeness sake,
>> I was suggesting making the option more general, applying to more to
>> more than just local-gdbinit (assuming more applications were readily
>> available).
>
> The warning is there duue to local-gdbinit (possible) deprecation.  No other
> file is being deprecated.
>
> To apply it for example to use of deprecated GDB commands (and not files)
> I somehow find too unrelated, I do not see any need / usefulness of such
> warning.

Sure.  I don't have an opinion, it was just a suggestion.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*
  2012-04-05 16:15     ` Eli Zaretskii
@ 2012-04-05 21:00       ` Jan Kratochvil
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Kratochvil @ 2012-04-05 21:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Thu, 05 Apr 2012 18:15:03 +0200, Eli Zaretskii wrote:
> But this hunk changes "on" into "yes", which is not what you seem to
> say will happen.  Did I understand correctly that GDB will print "on"?
+
> Whatever we do, I think these options should behave consistently.

I see there are continuous issues with the enum overload
of 'set auto-load local-gdbinit', therefore created a new boolean
'set auto-load local-gdbinit-warning' instead.

In fact one can find it easily as it is printed during
'set auto-load local-gdbinit<tab><tab>'
which was the original goal of the enum overload.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-04-05 21:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-29  9:13 [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
2012-03-29 20:58 ` Doug Evans
2012-03-30  6:28   ` Eli Zaretskii
2012-03-30  6:33     ` Doug Evans
2012-03-30  6:45       ` Jan Kratochvil
2012-03-30 18:08     ` suspend: " Jan Kratochvil
2012-03-30  7:26 ` Eli Zaretskii
2012-04-02 20:31   ` [doc patch] objfile -> @var{objfile} [Re: [patch#2 3/6] set auto-load local-gdbinit warn-and-*] Jan Kratochvil
2012-04-02 21:10     ` Eli Zaretskii
2012-04-02 21:13       ` Jan Kratochvil
2012-04-02 21:19         ` Eli Zaretskii
2012-04-03 18:33   ` [patch#2 3/6] set auto-load local-gdbinit warn-and-* Jan Kratochvil
2012-04-05 16:15     ` Eli Zaretskii
2012-04-05 21:00       ` Jan Kratochvil
2012-04-05 16:28     ` Doug Evans
2012-04-05 16:53       ` Jan Kratochvil
2012-04-05 17:03         ` Doug Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox