Re: [PATCH 2/6] kern/lock.h: quiet GCC warnings about set but unused variables

[Marin Ramesa]

On 15.12.2013 20:13:22, Samuel Thibault wrote: 
> > index 8fe3672..67067cf 100644
> > --- a/kern/task.c
> > +++ b/kern/task.c
> > @@ -769,7 +769,7 @@ kern_return_t task_info(
> > event_info->cow_faults = task->cow_faults;
> > event_info->messages_sent = task->messages_sent;
> > event_info->messages_received =
> > task->messages_received;
> > -   task_unlock(&task);
> > +   task_unlock(task);
> 
> Err, this is not the same at all, while task_lock is still using
> &task. Either both need the fix, or none.

They both need the fix. This code won't compile when simple_lock() is 
an unempty macro or a function.

Re: [PATCH 2/6] kern/lock.h: quiet GCC warnings about set but unused variables

[Samuel Thibault]

Marin Ramesa, le Thu 12 Dec 2013 18:27:04 +0100, a écrit :
> --- a/kern/lock.h
> +++ b/kern/lock.h
> @@ -94,7 +94,10 @@ extern voidcheck_simple_locks(void);
>  /*
>   * Do not allocate storage for locks if not needed.
>   */
> -#define  decl_simple_lock_data(class,name)
> +struct l {};

Use a better name than just "l" :)

> diff --git a/kern/sched_prim.c b/kern/sched_prim.c
> index c06cd77..71f32c5 100644
> --- a/kern/sched_prim.c
> +++ b/kern/sched_prim.c
> @@ -228,6 +228,8 @@ void assert_wait(
>   thread_tthread;
>  #if  MACH_SLOCKS
>   simple_lock_t   lock;
> +#else /* MACH_SLOCKS */
> + decl_simple_lock_data( , lock);

This is not the same: simple_lock_t is a pointer. It'd be better to just
find a way to use decl_simple_lock_data for this.

decl_simple_lock_data( , *lock);

would probably just work as expected.

> index 8fe3672..67067cf 100644
> --- a/kern/task.c
> +++ b/kern/task.c
> @@ -769,7 +769,7 @@ kern_return_t task_info(
>   event_info->cow_faults = task->cow_faults;
>   event_info->messages_sent = task->messages_sent;
>   event_info->messages_received = task->messages_received;
> - task_unlock(&task);
> + task_unlock(task);

Err, this is not the same at all, while task_lock is still using &task.
Either both need the fix, or none.

Samuel

[PATCH 2/6] kern/lock.h: quiet GCC warnings about set but unused variables

[Marin Ramesa]

This is a better version of these two patches:

http://lists.gnu.org/archive/html/bug-hurd/2013-12/msg6.html
http://lists.gnu.org/archive/html/bug-hurd/2013-12/msg00375.html

It uses an empty structure to represent a lock when !MACH_SLOCKS
and a modified simple_unlock() to quiet GCC warnings about set
but unused variables.

* kern/lock.h (l): Define.
(decl_simple_lock_data): Likewise.
(simple_unlock): Likewise.
* kern/sched_prim.c [!MACH_SLOCKS] (lock): Declare.
* kern/task.c (task_info) (task_unlock) (task): Remove address operator.

---
 kern/lock.h   | 9 ++---
 kern/sched_prim.c | 6 ++
 kern/task.c   | 2 +-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/kern/lock.h b/kern/lock.h
index 4f38ea3..a4e4d10 100644
--- a/kern/lock.h
+++ b/kern/lock.h
@@ -94,7 +94,10 @@ extern void  check_simple_locks(void);
 /*
  * Do not allocate storage for locks if not needed.
  */
-#definedecl_simple_lock_data(class,name)
+struct l {};
+#definedecl_simple_lock_data(class,name) \
+class struct l name;
+
 #definesimple_lock_addr(lock)  ((simple_lock_t)0)
 
 /*
@@ -102,8 +105,8 @@ extern void check_simple_locks(void);
  */
 #define simple_lock_init(l)
 #define simple_lock(l)
-#define simple_unlock(l)
-#define simple_lock_try(l) (TRUE)  /* always succeeds */
+#define simple_unlock(l) ((void)(l))
+#define simple_lock_try(l) (TRUE)  /* always succeeds */
 #define simple_lock_taken(l)   (1) /* always succeeds */
 #define check_simple_locks()
 #define simple_lock_pause()
diff --git a/kern/sched_prim.c b/kern/sched_prim.c
index c06cd77..71f32c5 100644
--- a/kern/sched_prim.c
+++ b/kern/sched_prim.c
@@ -228,6 +228,8 @@ void assert_wait(
thread_tthread;
 #ifMACH_SLOCKS
simple_lock_t   lock;
+#else /* MACH_SLOCKS */
+   decl_simple_lock_data( , lock);
 #endif /* MACH_SLOCKS */
spl_t   s;
 
@@ -286,6 +288,8 @@ void clear_wait(
queue_t q;
 #ifMACH_SLOCKS
simple_lock_t   lock;
+#else /* MACH_SLOCKS */
+   decl_simple_lock_data( , lock);
 #endif /* MACH_SLOCKS */
event_t event;
spl_t   s;
@@ -389,6 +393,8 @@ void thread_wakeup_prim(
thread_tthread, next_th;
 #ifMACH_SLOCKS
simple_lock_t   lock;
+#else /* MACH_SLOCKS */
+   decl_simple_lock_data( , lock);
 #endif  /* MACH_SLOCKS */
spl_t   s;
int state;
diff --git a/kern/task.c b/kern/task.c
index 8fe3672..67067cf 100644
--- a/kern/task.c
+++ b/kern/task.c
@@ -769,7 +769,7 @@ kern_return_t task_info(
event_info->cow_faults = task->cow_faults;
event_info->messages_sent = task->messages_sent;
event_info->messages_received = task->messages_received;
-   task_unlock(&task);
+   task_unlock(task);
 
*task_info_count = TASK_EVENTS_INFO_COUNT;
break;
-- 
1.8.1.4