Convert handmade 'max' to max().
Signed-off-by: Richard Knutsson <[email protected]>
---
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:
Convert m_ts ("message text size") from int to size_t.
Signed-off-by: Richard Knutsson <[email protected]>
---
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 */
Convert handmade 'min' to min().
Signed-off-by: Richard Knutsson <[email protected]>
---
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;
On Mon, 17 Dec 2007 03:35:55 +0100 (MET) Richard Knutsson <[email protected]> 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...
On Mon, 17 Dec 2007 03:36:00 +0100 (MET) Richard Knutsson <[email protected]> wrote:
> Convert m_ts ("message text size") from int to size_t.
>
> Signed-off-by: Richard Knutsson <[email protected]>
> ---
> 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.
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 <[email protected]> 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