Re: [OB] targe.c: extern reset_schedlock

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

Hui,

>  target_mourn_inferior (void)
>  {
>    struct target_ops *t;
> +  extern void reset_schedlock ();

This sort of local declaration should *not* be used except under
special circumstances.  Declarations inside a .c, a fortiori inside
a subprogram, are a maintenance hazard, because the compiler can
not check for you that this declaration matches the actual definition.
In other words, if someone changes the function profile, and forgets
to update this call site, then you'll have a program that still
compiles without warning, but whose execution becomes undefined.

Another tiny detail is the fact that functions without arguments
should have a "void" parameter, as opposed to nothing. In other
words, replace:

    extern void reset_schedlock ();

by:

    extern void reset_schedlock (void);

Attached is a patch I just checked in to correct both issues.
I also improved the original patch in terms of the comments.

-- 
Joel

[-- Attachment #2: reset_schedlock.diff --]
[-- Type: text/x-diff, Size: 2641 bytes --]

commit 59b929a962c497fdc4002817f65917966972a818
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Sun May 16 22:25:53 2010 -0700

    Add reset_schedlock declaration in target.h.
    
    This patches improves a couple of previous patches:
      - one that introduces reset_schedlock, but failed to add a declarationl;
      - one that was checked in to avoid a compilation failure due to that
        missing declaration.
    It also improves the declaration itself to better conform to our coding
    practices.  Same for the comments.
    
    2010-05-17  Joel Brobecker  <brobecker@adacore.com>
    
            * target.h (reset_schedlock): Add declaration.
            * infrun.c (reset_schedlock): Add missing void in function profile.
            * target.c (target_mourn_inferior): Delete local declaration of
            reset_schedlock.  Style-fix in comment.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cb272cd..5d35d00 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2010-05-17  Joel Brobecker  <brobecker@adacore.com>
+
+	* target.h (reset_schedlock): Add declaration.
+	* infrun.c (reset_schedlock): Add missing void in function profile.
+	* target.c (target_mourn_inferior): Delete local declaration of
+	reset_schedlock.  Style-fix in comment.
+
 2010-05-17  Hui Zhu  <teawater@gmail.com>
 
 	* target.c (target_mourn_inferior): Extern reset_schedlock.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 96da4cb..025ba0a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1420,10 +1420,11 @@ set_schedlock_func (char *args, int from_tty, struct cmd_list_element *c)
     }
 }
 
-/* reset_schedlock -- public */
+/* If SCHEDULER_MODE is on, then set it back to off.  Warn the user
+   about the change.  */
  
 void
-reset_schedlock ()
+reset_schedlock (void)
 {
   if (scheduler_mode == schedlock_on)
     {
diff --git a/gdb/target.c b/gdb/target.c
index cee3582..ebcf51c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2244,9 +2244,8 @@ void
 target_mourn_inferior (void)
 {
   struct target_ops *t;
-  extern void reset_schedlock ();
 
-  /* Clear schedlock in infrun.c */
+  /* Clear schedlock in infrun.c.  */
   reset_schedlock ();
 
   for (t = current_target.beneath; t != NULL; t = t->beneath)
diff --git a/gdb/target.h b/gdb/target.h
index d4bd007..cd2e82b 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -873,6 +873,8 @@ int target_write_memory_blocks (VEC(memory_write_request_s) *requests,
 
 /* From infrun.c.  */
 
+extern void reset_schedlock (void);
+
 extern int inferior_has_forked (ptid_t pid, ptid_t *child_pid);
 
 extern int inferior_has_vforked (ptid_t pid, ptid_t *child_pid);