[PATCH 1/3] ipc: Convert handmade 'max' to max().

[PATCH 1/3] ipc: Convert handmade 'max' to max().

[Richard Knutsson]

Convert handmade 'max' to max().

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---

 msg.c |    2 +-
 sem.c |    2 +-
 shm.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/ipc/msg.c b/ipc/msg.c
index fdf3db5..74d0709 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -473,7 +473,7 @@ asmlinkage long sys_msgctl(int msqid, int cmd, struct msqid_ds __user *buf)
 		up_read(&msg_ids(ns).rw_mutex);
 		if (copy_to_user(buf, &msginfo, sizeof(struct msginfo)))
 			return -EFAULT;
-		return (max_id < 0) ? 0 : max_id;
+		return max(max_id, 0);
 	}
 	case MSG_STAT:	/* msqid is an index rather than a msg queue id */
 	case IPC_STAT:
diff --git a/ipc/sem.c b/ipc/sem.c
index 35952c0..9ac00ac 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -641,7 +641,7 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, int semnum,
 		up_read(&sem_ids(ns).rw_mutex);
 		if (copy_to_user (arg.__buf, &seminfo, sizeof(struct seminfo))) 
 			return -EFAULT;
-		return (max_id < 0) ? 0: max_id;
+		return max(max_id, 0);
 	}
 	case SEM_STAT:
 	{
diff --git a/ipc/shm.c b/ipc/shm.c
index 3818fae..45e154a 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -704,7 +704,7 @@ asmlinkage long sys_shmctl (int shmid, int cmd, struct shmid_ds __user *buf)
 			goto out;
 		}
 
-		err = err < 0 ? 0 : err;
+		err = max(err, 0);
 		goto out;
 	}
 	case SHM_STAT:

[PATCH 2/3] msg.h: Convert m_ts from int to size_t.

[Richard Knutsson]

Convert m_ts ("message text size") from int to size_t.

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
Remove some trailing spaces, since we are in the neighborhood.


diff --git a/include/linux/msg.h b/include/linux/msg.h
index 10a3d5a..7a61952 100644
--- a/include/linux/msg.h
+++ b/include/linux/msg.h
@@ -67,8 +67,8 @@ struct msginfo {
 /* one msg_msg structure for each message */
 struct msg_msg {
 	struct list_head m_list; 
-	long  m_type;          
-	int m_ts;           /* message text size */
+	long  m_type;
+	size_t m_ts;           /* message text size */
 	struct msg_msgseg* next;
 	void *security;
 	/* the actual message follows immediately */

[PATCH 3/3] ipc: Convert handmade 'min' to min().

[Richard Knutsson]

Convert handmade 'min' to min().

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
Dependent on 'msg->m_ts' being changed from int to size_t.


diff --git a/ipc/msg.c b/ipc/msg.c
index fdf3db5..74d0709 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -920,7 +920,7 @@ out_unlock:
 	if (IS_ERR(msg))
 		return PTR_ERR(msg);
 
-	msgsz = (msgsz > msg->m_ts) ? msg->m_ts : msgsz;
+	msgsz = min(msgsz, msg->m_ts);
 	*pmtype = msg->m_type;
 	if (store_msg(mtext, msg, msgsz))
 		msgsz = -EFAULT;

Re: [PATCH 1/3] ipc: Convert handmade 'max' to max().

[Andrew Morton]

On Mon, 17 Dec 2007 03:35:55 +0100 (MET) Richard Knutsson <ricknu-0@student.ltu.se> wrote:

> Convert handmade 'max' to max().
> 
> ...
>
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -473,7 +473,7 @@ asmlinkage long sys_msgctl(int msqid, int cmd, struct msqid_ds __user *buf)
>  		up_read(&msg_ids(ns).rw_mutex);
>  		if (copy_to_user(buf, &msginfo, sizeof(struct msginfo)))
>  			return -EFAULT;
> -		return (max_id < 0) ? 0 : max_id;
> +		return max(max_id, 0);

I don't think I like that much.

I tend to think of max() as being an arithmetic sort of thing: pick the
largest of two scalars.

But the code which you're changing is a _logical_ operation.  It says "if
ipc_get_maxid() returned an error, then return zero.  Otherwise return
whatever ipc_get_maxid() returned".

Yes, max() will do the right thing here, but I think it's a bit of weird
trick?


I mean, if ipc_get_maxid() were a better function, it would return a -ve
errno when something failed rather than the present dopey hard-coded -1. 
In which case the code would read 

	return IS_ERR_VALUE(max_id) ? 0 : max_id;

in which case, converting it to max() would be even less appropriate.  If
you see what I mean...

Re: [PATCH 2/3] msg.h: Convert m_ts from int to size_t.

[Andrew Morton]

On Mon, 17 Dec 2007 03:36:00 +0100 (MET) Richard Knutsson <ricknu-0@student.ltu.se> wrote:

> Convert m_ts ("message text size") from int to size_t.
> 
> Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
> ---
> Remove some trailing spaces, since we are in the neighborhood.
> 
> 
> diff --git a/include/linux/msg.h b/include/linux/msg.h
> index 10a3d5a..7a61952 100644
> --- a/include/linux/msg.h
> +++ b/include/linux/msg.h
> @@ -67,8 +67,8 @@ struct msginfo {
>  /* one msg_msg structure for each message */
>  struct msg_msg {
>  	struct list_head m_list; 
> -	long  m_type;          
> -	int m_ts;           /* message text size */
> +	long  m_type;
> +	size_t m_ts;           /* message text size */
>  	struct msg_msgseg* next;
>  	void *security;
>  	/* the actual message follows immediately */

hm, spose so.  But if we're to do this then we'd need to fix qsize and
r_maxsize and various other things.  And we'd need to convert msg_bytes to
atomic_size_t, which would prove interesting ;)

So this is at best a partial conversion and the code in there does need
careful checking for the use of appropriate types.  Does this patch get us
partway toward the proper solution?  Dunno - someone would need to sit down
and work out what the best types are for all those related things.

Re: [PATCH 1/3] ipc: Convert handmade 'max' to max().

[Richard Knutsson]

Sorry for the late response, have been away during the holidays.

Andrew Morton wrote:
> On Mon, 17 Dec 2007 03:35:55 +0100 (MET) Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
>   
>> Convert handmade 'max' to max().
>>
>> ...
>>
>> --- a/ipc/msg.c
>> +++ b/ipc/msg.c
>> @@ -473,7 +473,7 @@ asmlinkage long sys_msgctl(int msqid, int cmd, struct msqid_ds __user *buf)
>>  		up_read(&msg_ids(ns).rw_mutex);
>>  		if (copy_to_user(buf, &msginfo, sizeof(struct msginfo)))
>>  			return -EFAULT;
>> -		return (max_id < 0) ? 0 : max_id;
>> +		return max(max_id, 0);
>>     
>
> I don't think I like that much.
>
> I tend to think of max() as being an arithmetic sort of thing: pick the
> largest of two scalars.
>
> But the code which you're changing is a _logical_ operation.  It says "if
> ipc_get_maxid() returned an error, then return zero.  Otherwise return
> whatever ipc_get_maxid() returned".
>
> Yes, max() will do the right thing here, but I think it's a bit of weird
> trick?
>
>
> I mean, if ipc_get_maxid() were a better function, it would return a -ve
> errno when something failed rather than the present dopey hard-coded -1. 
> In which case the code would read 
>
> 	return IS_ERR_VALUE(max_id) ? 0 : max_id;
>
> in which case, converting it to max() would be even less appropriate.  If
> you see what I mean...
>   
Yes, have to agree. Were too quick with the changing...

Thanks
Richard Knutsson