On Wed, 2011-09-14 at 18:35 +0200, Karel Zak wrote:
> On Wed, Sep 14, 2011 at 10:21:53AM -0300, Davidlohr Bueso wrote:
> > On Tue, 2011-09-13 at 23:13 +0200, Sami Kerola wrote:
> > Please try the following patch, it fixed it for me. In the case when the
> > errors were shown, the msgctl(2) was returning 0 (success), so errno
> > shouldn't be checked. Remember, this isn't reseted afterwards, so we are
> > running into bogus messages (the msgid's are being correctly deleted).
> >
> > Thanks,
> > Davidlohr
> >
> > From 952d6100c28505005c0a335e3e64f384bd941859 Mon Sep 17 00:00:00 2001
> > From: Davidlohr Bueso <dave@xxxxxxx>
> > Date: Wed, 14 Sep 2011 10:17:15 -0300
> > Subject: [PATCH] ipcrm: check IPC syscalls
> >
> > It's not enough to check errno for errors as the variable is not reset, we also need to check the last syscall return value to verify a problem.
> > This addresses bogus msgqueue errors when deleting keys.
> >
> > Signed-off-by: Davidlohr Bueso <dave@xxxxxxx>
> > ---
> > sys-utils/ipcrm.c | 9 +++++----
> > 1 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/sys-utils/ipcrm.c b/sys-utils/ipcrm.c
> > index a8d6623..c794cbe 100644
> > --- a/sys-utils/ipcrm.c
> > +++ b/sys-utils/ipcrm.c
> > @@ -65,6 +65,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
> >
> > int remove_id(int type, int iskey, int id)
> > {
> > + int ret;
> > char *errmsg;
> > /* needed to delete semaphores */
> > union semun arg;
> > @@ -75,24 +76,24 @@ int remove_id(int type, int iskey, int id)
> > case SHM:
> > if (verbose)
> > printf(_("removing shared memory segment id `%d'\n"), id);
> > - shmctl(id, IPC_RMID, NULL);
> > + ret = shmctl(id, IPC_RMID, NULL);
> > break;
> > case MSG:
> > if (verbose)
> > printf(_("removing message queue id `%d'\n"), id);
> > - msgctl(id, IPC_RMID, NULL);
> > + ret = msgctl(id, IPC_RMID, NULL);
> > break;
> > case SEM:
> > if (verbose)
> > printf(_("removing semaphore id `%d'\n"), id);
> > - semctl(id, 0, IPC_RMID, arg);
> > + ret = semctl(id, 0, IPC_RMID, arg);
> > break;
> > default:
> > errx(EXIT_FAILURE, "impossible occurred");
> > }
> >
> > /* how did the removal go? */
> > - switch (errno) {
> > + switch (errno && ret) {
>
> so true/false only ... why we need the "case EACCES" (etc.)?
>
> > case 0:
> > return 0;
> > case EACCES:
>
> why we cannot use:
>
> if (ret < 0)
> switch(errno) {
> case EACCES:
> ...
> break;
>
> ....
> }
Well, yes, the case 0 didn't make much sense, otherwise I don't really
care how we evaluate ret/errno. Is the below patch ok with you?
From: Davidlohr Bueso <dave@xxxxxxx>
Date: Wed, 14 Sep 2011 14:02:15 -0300
Subject: [PATCH] ipcrm: check IPC syscalls
It's not enough to check errno for errors as the variable is not reset, we also need to check the last syscall return value to verify a problem.
This addresses bogus msgqueue errors when deleting keys.
Signed-off-by: Davidlohr Bueso <dave@xxxxxxx>
---
sys-utils/ipcrm.c | 51 ++++++++++++++++++++++++++-------------------------
1 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/sys-utils/ipcrm.c b/sys-utils/ipcrm.c
index a8d6623..a176fc7 100644
--- a/sys-utils/ipcrm.c
+++ b/sys-utils/ipcrm.c
@@ -65,6 +65,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
int remove_id(int type, int iskey, int id)
{
+ int ret;
char *errmsg;
/* needed to delete semaphores */
union semun arg;
@@ -75,46 +76,46 @@ int remove_id(int type, int iskey, int id)
case SHM:
if (verbose)
printf(_("removing shared memory segment id `%d'\n"), id);
- shmctl(id, IPC_RMID, NULL);
+ ret = shmctl(id, IPC_RMID, NULL);
break;
case MSG:
if (verbose)
printf(_("removing message queue id `%d'\n"), id);
- msgctl(id, IPC_RMID, NULL);
+ ret = msgctl(id, IPC_RMID, NULL);
break;
case SEM:
if (verbose)
printf(_("removing semaphore id `%d'\n"), id);
- semctl(id, 0, IPC_RMID, arg);
+ ret = semctl(id, 0, IPC_RMID, arg);
break;
default:
errx(EXIT_FAILURE, "impossible occurred");
}
/* how did the removal go? */
- switch (errno) {
- case 0:
- return 0;
- case EACCES:
- case EPERM:
- errmsg = iskey ? _("permission denied for key")
- : _("permission denied for id");
- break;
- case EINVAL:
- errmsg = iskey ? _("invalid key")
- : _("invalid id");
- break;
- case EIDRM:
- errmsg = iskey ? _("already removed key")
- : _("already removed id");
- break;
- default:
- if (iskey)
- err(EXIT_FAILURE, _("key failed"));
- else
- err(EXIT_FAILURE, _("id failed"));
+ if (ret < 0) {
+ switch (errno) {
+ case EACCES:
+ case EPERM:
+ errmsg = iskey ? _("permission denied for key")
+ : _("permission denied for id");
+ break;
+ case EINVAL:
+ errmsg = iskey ? _("invalid key")
+ : _("invalid id");
+ break;
+ case EIDRM:
+ errmsg = iskey ? _("already removed key")
+ : _("already removed id");
+ break;
+ default:
+ if (iskey)
+ err(EXIT_FAILURE, _("key failed"));
+ else
+ err(EXIT_FAILURE, _("id failed"));
+ }
+ warnx("%s (%d)", errmsg, id);
}
- warnx("%s (%d)", errmsg, id);
return 1;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
