* [PATCH 01/17] libuuid: add extern qualifiers to uuid/uuid.h system header
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 02/17] include: simplify fputc_careful() in carefulputc.h Sami Kerola
` (17 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
libuuid/src/uuid.h | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/libuuid/src/uuid.h b/libuuid/src/uuid.h
index 874d65a..30bd4c0 100644
--- a/libuuid/src/uuid.h
+++ b/libuuid/src/uuid.h
@@ -67,35 +67,35 @@ extern "C" {
#endif
/* clear.c */
-void uuid_clear(uuid_t uu);
+extern void uuid_clear(uuid_t uu);
/* compare.c */
-int uuid_compare(const uuid_t uu1, const uuid_t uu2);
+extern int uuid_compare(const uuid_t uu1, const uuid_t uu2);
/* copy.c */
-void uuid_copy(uuid_t dst, const uuid_t src);
+extern void uuid_copy(uuid_t dst, const uuid_t src);
/* gen_uuid.c */
-void uuid_generate(uuid_t out);
-void uuid_generate_random(uuid_t out);
-void uuid_generate_time(uuid_t out);
-int uuid_generate_time_safe(uuid_t out);
+extern void uuid_generate(uuid_t out);
+extern void uuid_generate_random(uuid_t out);
+extern void uuid_generate_time(uuid_t out);
+extern int uuid_generate_time_safe(uuid_t out);
/* isnull.c */
-int uuid_is_null(const uuid_t uu);
+extern int uuid_is_null(const uuid_t uu);
/* parse.c */
-int uuid_parse(const char *in, uuid_t uu);
+extern int uuid_parse(const char *in, uuid_t uu);
/* unparse.c */
-void uuid_unparse(const uuid_t uu, char *out);
-void uuid_unparse_lower(const uuid_t uu, char *out);
-void uuid_unparse_upper(const uuid_t uu, char *out);
+extern void uuid_unparse(const uuid_t uu, char *out);
+extern void uuid_unparse_lower(const uuid_t uu, char *out);
+extern void uuid_unparse_upper(const uuid_t uu, char *out);
/* uuid_time.c */
-time_t uuid_time(const uuid_t uu, struct timeval *ret_tv);
-int uuid_type(const uuid_t uu);
-int uuid_variant(const uuid_t uu);
+extern time_t uuid_time(const uuid_t uu, struct timeval *ret_tv);
+extern int uuid_type(const uuid_t uu);
+extern int uuid_variant(const uuid_t uu);
#ifdef __cplusplus
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/17] include: simplify fputc_careful() in carefulputc.h
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
2014-09-07 12:42 ` [PATCH 01/17] libuuid: add extern qualifiers to uuid/uuid.h system header Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 03/17] lib: avoid use of obsolete getpass() function Sami Kerola
` (16 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
New version of comparisions does the same thing as the old without being
quite as difficult to understand.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/carefulputc.h | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/include/carefulputc.h b/include/carefulputc.h
index f29dc69..c116d0a 100644
--- a/include/carefulputc.h
+++ b/include/carefulputc.h
@@ -10,26 +10,22 @@
#include <string.h>
#include <ctype.h>
-#define iso8859x_iscntrl(c) \
- (((c) & 0x7f) < 0x20 || (c) == 0x7f)
-
-static inline int fputc_careful(int c, FILE *fp, const char fail) {
+static inline int fputc_careful(int c, FILE *fp, const char fail)
+{
int ret;
- if (c == '\007' || c == '\t' || c == '\r' || c == '\n' ||
- (!iso8859x_iscntrl(c) && (isprint(c) || isspace(c))))
+ if (isprint(c) || c == '\a' || c == '\t' || c == '\r' || c == '\n')
ret = putc(c, fp);
- else if ((c & 0x80) || !isprint(c^0x40))
- ret = fprintf(fp, "\\%3o", (unsigned char) c);
+ else if (!isascii(c))
+ ret = fprintf(fp, "\\%3o", (unsigned char)c);
else {
ret = putc(fail, fp);
if (ret != EOF)
- ret = putc(c^0x40, fp);
+ ret = putc(c ^ 0x40, fp);
}
return (ret < 0) ? EOF : 0;
}
-
static inline void fputs_quoted(const char *data, FILE *out)
{
const char *p;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/17] lib: avoid use of obsolete getpass() function
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
2014-09-07 12:42 ` [PATCH 01/17] libuuid: add extern qualifiers to uuid/uuid.h system header Sami Kerola
2014-09-07 12:42 ` [PATCH 02/17] include: simplify fputc_careful() in carefulputc.h Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-12 8:31 ` Karel Zak
2014-09-07 12:42 ` [PATCH 04/17] lib: add function to remove string from memory Sami Kerola
` (15 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Reference: http://man7.org/linux/man-pages/man3/getpass.3.html
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/xgetpass.h | 2 +-
lib/xgetpass.c | 56 +++++++++++++++++++++++++-----------------------------
2 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/include/xgetpass.h b/include/xgetpass.h
index b5a3c87..622ba8c 100644
--- a/include/xgetpass.h
+++ b/include/xgetpass.h
@@ -1,6 +1,6 @@
#ifndef UTIL_LINUX_XGETPASS_H
#define UTIL_LINUX_XGETPASS_H
-extern char *xgetpass(int pfd, const char *prompt);
+extern char *xgetpass(FILE *input, const char *prompt);
#endif /* UTIL_LINUX_XGETPASS_H */
diff --git a/lib/xgetpass.c b/lib/xgetpass.c
index ba20894..9c7d431 100644
--- a/lib/xgetpass.c
+++ b/lib/xgetpass.c
@@ -5,42 +5,38 @@
* Public domain.
*/
+#include <err.h>
#include <stdio.h>
-#include <string.h>
#include <stdlib.h>
+#include <termios.h>
#include <unistd.h>
-#include <sys/ioctl.h>
-#include <sys/stat.h>
-#include "c.h"
-#include "xgetpass.h"
-
-char *xgetpass(int pfd, const char *prompt)
+char *xgetpass(FILE *input, const char *prompt)
{
char *pass = NULL;
- int len = 0, i;
-
- if (pfd < 0) /* terminal */
- return getpass(prompt);
-
- for (i=0; ; i++) {
- if (i >= len-1) {
- char *tmppass = pass;
- len += 128;
-
- pass = realloc(tmppass, len);
- if (!pass) {
- pass = tmppass; /* the old buffer hasn't changed */
- break;
- }
- }
- if (pass && (read(pfd, pass + i, 1) != 1 ||
- pass[i] == '\n' || pass[i] == 0))
- break;
+ struct termios saved, no_echo;
+ const int fd = fileno(input);
+ size_t dummy = 0;
+ ssize_t len;
+
+ fputs(prompt, stdout);
+ if (isatty(fd)) {
+ /* disable echo */
+ tcgetattr(fd, &saved);
+ no_echo = saved;
+ no_echo.c_lflag &= ~ECHO;
+ no_echo.c_lflag |= ECHONL;
+ if (tcsetattr(fd, TCSANOW, &no_echo))
+ err(EXIT_FAILURE, "could not set terminal attributes");
}
-
- if (pass)
- pass[i] = '\0';
+ len = getline(&pass, &dummy, input);
+ if (len < 0)
+ warn("could not getline");
+ if (*(pass + len - 1) == '\n')
+ *(pass + len - 1) = '\0';
+ if (isatty(fd))
+ /* restore terminal */
+ if (tcsetattr(fd, TCSANOW, &saved))
+ err(EXIT_FAILURE, "could not set terminal attributes");
return pass;
}
-
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/17] lib: add function to remove string from memory
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (2 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 03/17] lib: avoid use of obsolete getpass() function Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation Sami Kerola
` (14 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
The memcpy(3) can be get optimized out by compiler.
Reference: https://www.securecoding.cert.org/confluence/display/seccode/MSC06-C.+Beware+of+compiler+optimizations
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/xgetpass.h | 1 +
lib/xgetpass.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/xgetpass.h b/include/xgetpass.h
index 622ba8c..0f77419 100644
--- a/include/xgetpass.h
+++ b/include/xgetpass.h
@@ -2,5 +2,6 @@
#define UTIL_LINUX_XGETPASS_H
extern char *xgetpass(FILE *input, const char *prompt);
+extern int memset_s(void *v, size_t sz, int c);
#endif /* UTIL_LINUX_XGETPASS_H */
diff --git a/lib/xgetpass.c b/lib/xgetpass.c
index 9c7d431..0948ff1 100644
--- a/lib/xgetpass.c
+++ b/lib/xgetpass.c
@@ -6,11 +6,14 @@
*/
#include <err.h>
+#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <termios.h>
#include <unistd.h>
+#include "xgetpass.h"
+
char *xgetpass(FILE *input, const char *prompt)
{
char *pass = NULL;
@@ -40,3 +43,18 @@ char *xgetpass(FILE *input, const char *prompt)
err(EXIT_FAILURE, "could not set terminal attributes");
return pass;
}
+
+/* Ensure memory is set to value c without compiler optimization getting
+ * into way, that could happen with memset(3). This function can be used
+ * for example getting rid of in memory password strings read with
+ * xgetpass(). */
+ int memset_s(void *v, size_t sz, int c)
+{
+ volatile unsigned char *p = v;
+
+ if (v == NULL)
+ return EINVAL;
+ while (sz--)
+ *p++ = c;
+ return 0;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (3 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 04/17] lib: add function to remove string from memory Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-12 8:44 ` Karel Zak
2014-09-07 12:42 ` [PATCH 06/17] last: make is_phantom() when kernel config does not include audit support Sami Kerola
` (13 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
login-utils/newgrp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index 55dad1b..84e20b0 100644
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -99,9 +99,10 @@ static int allow_setgid(struct passwd *pe, struct group *ge)
if (!(pwd = get_gshadow_pwd(ge->gr_name)))
pwd = ge->gr_passwd;
- if (pwd && *pwd && (xpwd = getpass(_("Password: ")))) {
+ if (pwd && *pwd && (xpwd = xgetpass(_("Password: ")))) {
char *cbuf = crypt(xpwd, pwd);
+ memset_s(xpwd, strlen(xpwd), 0);
if (!cbuf)
warn(_("crypt() failed"));
else if (strcmp(pwd, cbuf) == 0)
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation
2014-09-07 12:42 ` [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation Sami Kerola
@ 2014-09-12 8:44 ` Karel Zak
2014-09-16 20:33 ` Sami Kerola
0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2014-09-12 8:44 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Sep 07, 2014 at 01:42:53PM +0100, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> login-utils/newgrp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
If we really want to support passwords for groups then it would be
better to add support for this to PAM. But it seems it's so crazy and
unnecessary that nobody has been motivated to do this change in last
20 years.
It would be better to remove support for /etc/gshadow from newgrp at
all.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation
2014-09-12 8:44 ` Karel Zak
@ 2014-09-16 20:33 ` Sami Kerola
0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-16 20:33 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sun, Sep 07, 2014 at 01:42:53PM +0100, Sami Kerola wrote:
>> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
>> ---
>> login-utils/newgrp.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> If we really want to support passwords for groups then it would be
> better to add support for this to PAM. But it seems it's so crazy and
> unnecessary that nobody has been motivated to do this change in last
> 20 years.
>
> It would be better to remove support for /etc/gshadow from newgrp at
> all.
Either PAM does not support group passwords, or I misread docs & git
checkout. What comes to gshadow itself I do agree finding an example when
using them would be useful is hard. Then again the support for group
passwords has been around for long time and even opengroup mentions
authorizations.
http://pubs.opengroup.org/onlinepubs/009695299/utilities/newgrp.html
So that in mind I changed the newgrp change somewhat. Here is first
alteration.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Sun, 14 Sep 2014 17:29:54 +0100
Subject: [PATCH 15/17] newgrp: use libc function to read gshadow if it is available
The glib versionf of getsgnam() is using /etc/nsswitch.conf, allowing the
group passwords to come from external database.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
configure.ac | 1 +
login-utils/newgrp.c | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/configure.ac b/configure.ac
index 5b558ec..c84814f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -302,6 +302,7 @@ AC_CHECK_FUNCS([ \
getexecname \
getmntinfo \
getrlimit \
+ getsgnam \
inotify_init \
inotify_init1 \
jrand48 \
diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index 55dad1b..d492f23 100644
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -28,6 +28,10 @@
# include <crypt.h>
#endif
+#ifdef HAVE_GETSGNAM
+# include <gshadow.h>
+#endif
+
#include "c.h"
#include "closestream.h"
#include "nls.h"
@@ -37,6 +41,12 @@
/* try to read password from gshadow */
static char *get_gshadow_pwd(char *groupname)
{
+#ifdef HAVE_GETSGNAM
+ struct sgrp *sgrp;
+
+ sgrp = getsgnam(groupname);
+ return sgrp ? xstrdup(sgrp->sg_passwd) : NULL;
+#else
char buf[BUFSIZ];
char *pwd = NULL;
FILE *f;
@@ -69,6 +79,7 @@ static char *get_gshadow_pwd(char *groupname)
}
fclose(f);
return pwd ? xstrdup(pwd) : NULL;
+#endif /* HAVE_GETSGNAM */
}
static int allow_setgid(struct passwd *pe, struct group *ge)
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 06/17] last: make is_phantom() when kernel config does not include audit support
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (4 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 05/17] newgrp: use xgetpass() and memset_s() to group password validation Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 07/17] last: improve code readability by renaming variable names Sami Kerola
` (12 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
When kernel CONFIG_AUDIT is not set the /proc/<pid>/loginuid information
is not present resulting live sessions to be marked 'gone - no logout' in
last(1) print out. To go-around this change makes last(1) to look
/dev/<tty> device ownership as a substitute of loginuid.
The go-around seems to work fairly well, but it has it short comings.
For example after closing a X window session the /dev/ttyN file seems to
be owned by root, not the user who had it before entering to the X
session. While that is suboptimal it is still better than an attmempt to
determine uid_t by looking owner of the /proc/<struct utmp ut_pid>, that
is a login(1) process running as root.
The issue was found using Archlinux installation.
$ pacman -Qi linux
Name : linux
Version : 3.16-2
[...]
Build Date : Mon Aug 4 18:06:51 2014
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
login-utils/last.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/login-utils/last.c b/login-utils/last.c
index f7a2576..fb82f31 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -582,8 +582,6 @@ static int is_phantom(const struct last_control *ctl, struct utmp *ut)
{
struct passwd *pw;
char path[32];
- FILE *f = NULL;
- unsigned int loginuid;
int ret = 0;
if (ut->UL_UT_TIME < ctl->boot_time.tv_sec)
@@ -592,14 +590,26 @@ static int is_phantom(const struct last_control *ctl, struct utmp *ut)
if (!pw)
return 1;
sprintf(path, "/proc/%u/loginuid", ut->ut_pid);
- if (access(path, R_OK) != 0 || !(f = fopen(path, "r")))
- return 1;
+ if (access(path, R_OK) == 0) {
+ unsigned int loginuid;
+ FILE *f = NULL;
+
+ if (!(f = fopen(path, "r")))
+ return 1;
+ if (fscanf(f, "%u", &loginuid) != 1)
+ ret = 1;
+ fclose(f);
+ if (!ret && pw->pw_uid != loginuid)
+ return 1;
+ } else {
+ struct stat st;
- if (fscanf(f, "%u", &loginuid) != 1)
- ret = 1;
- fclose(f);
- if (!ret && pw->pw_uid != loginuid)
- return 1;
+ sprintf(path, "/dev/%s", ut->ut_line);
+ if (stat(path, &st))
+ return 1;
+ if (pw->pw_uid != st.st_uid)
+ return 1;
+ }
return ret;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/17] last: improve code readability by renaming variable names
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (5 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 06/17] last: make is_phantom() when kernel config does not include audit support Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 08/17] zramctl: fix two format string warnings Sami Kerola
` (11 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
login-utils/last.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/login-utils/last.c b/login-utils/last.c
index fb82f31..54d9e1f 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -376,9 +376,9 @@ static void trim_trailing_spaces(char *s)
/*
* Show one line of information on screen
*/
-static int list(const struct last_control *ctl, struct utmp *p, time_t t, int what)
+static int list(const struct last_control *ctl, struct utmp *p, time_t logout_time, int what)
{
- time_t secs, tmp, epoch;
+ time_t secs, utmp_time, now;
char logintime[LAST_TIMESTAMP_LEN];
char logouttime[LAST_TIMESTAMP_LEN];
char length[LAST_TIMESTAMP_LEN];
@@ -417,22 +417,25 @@ static int list(const struct last_control *ctl, struct utmp *p, time_t t, int wh
/*
* Calculate times
*/
- tmp = p->UL_UT_TIME;
+ utmp_time = p->UL_UT_TIME;
- if (ctl->present && (ctl->present < tmp || (0 < t && t < ctl->present)))
- return 0;
-
- if (time_formatter(ctl, &logintime[0], sizeof(logintime), &tmp, 0) < 0 ||
- time_formatter(ctl, &logouttime[0], sizeof(logouttime), &t, 1) < 0)
+ if (ctl->present) {
+ if (ctl->present < utmp_time)
+ return 0;
+ if (0 < logout_time && logout_time < ctl->present)
+ return 0;
+ }
+ if (time_formatter(ctl, &logintime[0], sizeof(logintime), &utmp_time, 0) < 0 ||
+ time_formatter(ctl, &logouttime[0], sizeof(logouttime), &logout_time, 1) < 0)
errx(EXIT_FAILURE, _("preallocation size exceeded"));
- secs = t - p->UL_UT_TIME;
+ secs = logout_time - utmp_time;
mins = (secs / 60) % 60;
hours = (secs / 3600) % 24;
days = secs / 86400;
- epoch = time(NULL);
- if (t == epoch) {
+ now = time(NULL);
+ if (logout_time == now) {
if (ctl->time_fmt > LAST_TIMEFTM_SHORT_CTIME) {
sprintf(logouttime, " still running");
length[0] = 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/17] zramctl: fix two format string warnings
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (6 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 07/17] last: improve code readability by renaming variable names Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 09/17] mountpoint: add struct mountpoint_control Sami Kerola
` (10 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Both warnings were similar.
sys-utils/zramctl.c:527:23: warning: format string is not a string
literal (potentially insecure) [-Wformat-security]
err(EXIT_FAILURE, zram->devname);
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/zramctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sys-utils/zramctl.c b/sys-utils/zramctl.c
index 961dba1..786be06 100644
--- a/sys-utils/zramctl.c
+++ b/sys-utils/zramctl.c
@@ -524,7 +524,7 @@ int main(int argc, char **argv)
if (optind < argc) {
zram = new_zram(argv[optind++]);
if (!zram_exist(zram))
- err(EXIT_FAILURE, zram->devname);
+ err(EXIT_FAILURE, "%s", zram->devname);
}
status(zram);
free_zram(zram);
@@ -560,7 +560,7 @@ int main(int argc, char **argv)
else {
zram = new_zram(argv[optind]);
if (!zram_exist(zram))
- err(EXIT_FAILURE, zram->devname);
+ err(EXIT_FAILURE, "%s", zram->devname);
}
if (zram_set_u64parm(zram, "reset", 1))
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/17] mountpoint: add struct mountpoint_control
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (7 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 08/17] zramctl: fix two format string warnings Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-12 9:14 ` Karel Zak
2014-09-07 12:42 ` [PATCH 10/17] mkfs.minix: fix couple compiler warnings Sami Kerola
` (9 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
This unifies variable names in different functions, and removes redundant
stat() calls.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/mountpoint.c | 86 ++++++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 45 deletions(-)
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index 3919ab7..3392cdd 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -40,9 +40,17 @@
#include "closestream.h"
#include "pathnames.h"
-static int quiet;
+struct mountpoint_control {
+ char *devname;
+ dev_t dev;
+ struct stat st;
+ uint8_t
+ dev_devno:1,
+ fs_devno:1,
+ quiet:1;
+};
-static int dir_to_device(const char *spec, dev_t *dev)
+static int dir_to_device(struct mountpoint_control *ctl)
{
struct libmnt_table *tb = mnt_new_table_from_file(_PATH_PROC_MOUNTINFO);
struct libmnt_fs *fs;
@@ -54,16 +62,13 @@ static int dir_to_device(const char *spec, dev_t *dev)
* Fallback. Traditional way to detect mountpoints. This way
* is independent on /proc, but not able to detect bind mounts.
*/
- struct stat pst, st;
+ struct stat pst;
char buf[PATH_MAX], *cn;
int len;
- if (stat(spec, &st) != 0)
- return -1;
-
- cn = mnt_resolve_path(spec, NULL); /* canonicalize */
+ cn = mnt_resolve_path(ctl->devname, NULL); /* canonicalize */
- len = snprintf(buf, sizeof(buf), "%s/..", cn ? cn : spec);
+ len = snprintf(buf, sizeof(buf), "%s/..", cn ? cn : ctl->devname);
free(cn);
if (len < 0 || (size_t) len + 1 > sizeof(buf))
@@ -71,9 +76,9 @@ static int dir_to_device(const char *spec, dev_t *dev)
if (stat(buf, &pst) !=0)
return -1;
- if ((st.st_dev != pst.st_dev) ||
- (st.st_dev == pst.st_dev && st.st_ino == pst.st_ino)) {
- *dev = st.st_dev;
+ if ((ctl->st.st_dev != pst.st_dev) ||
+ (ctl->st.st_dev == pst.st_dev && ctl->st.st_ino == pst.st_ino)) {
+ ctl->dev = ctl->st.st_dev;
return 0;
}
@@ -85,9 +90,9 @@ static int dir_to_device(const char *spec, dev_t *dev)
mnt_table_set_cache(tb, cache);
mnt_unref_cache(cache);
- fs = mnt_table_find_target(tb, spec, MNT_ITER_BACKWARD);
+ fs = mnt_table_find_target(tb, ctl->devname, MNT_ITER_BACKWARD);
if (fs && mnt_fs_get_target(fs)) {
- *dev = mnt_fs_get_devno(fs);
+ ctl->dev = mnt_fs_get_devno(fs);
rc = 0;
}
@@ -95,20 +100,14 @@ static int dir_to_device(const char *spec, dev_t *dev)
return rc;
}
-static int print_devno(const char *devname, struct stat *st)
+static int print_devno(const struct mountpoint_control *ctl)
{
- struct stat stbuf;
-
- if (!st && stat(devname, &stbuf) == 0)
- st = &stbuf;
- if (!st)
- return -1;
- if (!S_ISBLK(st->st_mode)) {
- if (!quiet)
- warnx(_("%s: not a block device"), devname);
+ if (!S_ISBLK(ctl->st.st_mode)) {
+ if (!ctl->quiet)
+ warnx(_("%s: not a block device"), ctl->devname);
return -1;
}
- printf("%u:%u\n", major(st->st_rdev), minor(st->st_rdev));
+ printf("%u:%u\n", major(ctl->st.st_rdev), minor(ctl->st.st_rdev));
return 0;
}
@@ -133,9 +132,8 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
int main(int argc, char **argv)
{
- int c, fs_devno = 0, dev_devno = 0, rc = 0;
- char *spec;
- struct stat st;
+ int c, rc = 0;
+ struct mountpoint_control ctl = { 0 };
static const struct option longopts[] = {
{ "quiet", 0, 0, 'q' },
@@ -157,13 +155,13 @@ int main(int argc, char **argv)
switch(c) {
case 'q':
- quiet = 1;
+ ctl.quiet = 1;
break;
case 'd':
- fs_devno = 1;
+ ctl.fs_devno = 1;
break;
case 'x':
- dev_devno = 1;
+ ctl.dev_devno = 1;
break;
case 'h':
usage(stdout);
@@ -180,27 +178,25 @@ int main(int argc, char **argv)
if (optind + 1 != argc)
usage(stderr);
- spec = argv[optind++];
+ ctl.devname = argv[optind];
- if (stat(spec, &st)) {
- if (!quiet)
- err(EXIT_FAILURE, "%s", spec);
+ if (stat(ctl.devname, &ctl.st)) {
+ if (!ctl.quiet)
+ err(EXIT_FAILURE, "%s", ctl.devname);
return EXIT_FAILURE;
}
- if (dev_devno)
- rc = print_devno(spec, &st);
+ if (ctl.dev_devno)
+ rc = print_devno(&ctl);
else {
- dev_t src;
-
- if ( dir_to_device(spec, &src)) {
- if (!quiet)
- printf(_("%s is not a mountpoint\n"), spec);
+ if (dir_to_device(&ctl)) {
+ if (!ctl.quiet)
+ printf(_("%s is not a mountpoint\n"), ctl.devname);
return EXIT_FAILURE;
}
- if (fs_devno)
- printf("%u:%u\n", major(src), minor(src));
- else if (!quiet)
- printf(_("%s is a mountpoint\n"), spec);
+ if (ctl.fs_devno)
+ printf("%u:%u\n", major(ctl.dev), minor(ctl.dev));
+ else if (!ctl.quiet)
+ printf(_("%s is a mountpoint\n"), ctl.devname);
}
return rc ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 09/17] mountpoint: add struct mountpoint_control
2014-09-07 12:42 ` [PATCH 09/17] mountpoint: add struct mountpoint_control Sami Kerola
@ 2014-09-12 9:14 ` Karel Zak
2014-09-16 20:36 ` Sami Kerola
0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2014-09-12 9:14 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Sep 07, 2014 at 01:42:57PM +0100, Sami Kerola wrote:
> This unifies variable names in different functions, and removes redundant
> stat() calls.
>
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> sys-utils/mountpoint.c | 86 ++++++++++++++++++++++++--------------------------
> 1 file changed, 41 insertions(+), 45 deletions(-)
>
> diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
> index 3919ab7..3392cdd 100644
> --- a/sys-utils/mountpoint.c
> +++ b/sys-utils/mountpoint.c
> @@ -40,9 +40,17 @@
> #include "closestream.h"
> #include "pathnames.h"
>
> -static int quiet;
> +struct mountpoint_control {
> + char *devname;
This member name makes the code difficult to read, on many places it's
not device at all. Please, use "path" or "spec" rather then "devname".
> + dev_t dev;
> + struct stat st;
> + uint8_t
> + dev_devno:1,
> + fs_devno:1,
> + quiet:1;
> +};
Use "unsigned int" for bit fields, it's compiler business to optimize
how the bits are packed in the struct.
C99, 6.7.2.1 Structure and union specifiers
A bit-field shall have a type that is a qualified or unqualified
version of _Bool, signed int, unsigned int, or some other
implementation-defined type.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 09/17] mountpoint: add struct mountpoint_control
2014-09-12 9:14 ` Karel Zak
@ 2014-09-16 20:36 ` Sami Kerola
0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-16 20:36 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sun, Sep 07, 2014 at 01:42:57PM +0100, Sami Kerola wrote:
>> This unifies variable names in different functions, and removes redundant
>> stat() calls.
>>
>> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
>> ---
>> sys-utils/mountpoint.c | 86 ++++++++++++++++++++++++--------------------------
>> 1 file changed, 41 insertions(+), 45 deletions(-)
>>
>> diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
>> index 3919ab7..3392cdd 100644
>> --- a/sys-utils/mountpoint.c
>> +++ b/sys-utils/mountpoint.c
>> @@ -40,9 +40,17 @@
>> #include "closestream.h"
>> #include "pathnames.h"
>>
>> -static int quiet;
>> +struct mountpoint_control {
>> + char *devname;
>
> This member name makes the code difficult to read, on many places it's
> not device at all. Please, use "path" or "spec" rather then "devname".
Done.
>> + dev_t dev;
>> + struct stat st;
>> + uint8_t
>> + dev_devno:1,
>> + fs_devno:1,
>> + quiet:1;
>> +};
>
> Use "unsigned int" for bit fields, it's compiler business to optimize
> how the bits are packed in the struct.
>
>
> C99, 6.7.2.1 Structure and union specifiers
>
> A bit-field shall have a type that is a qualified or unqualified
> version of _Bool, signed int, unsigned int, or some other
> implementation-defined type.
Thanks Karel. I'll stick to unsigned ints in future when doing bit fields.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Fri, 22 Aug 2014 13:25:48 +0300
Subject: [PATCH 07/17] mountpoint: add struct mountpoint_control
This unifies variable names in different functions, and removes redundant
stat() calls.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/mountpoint.c | 86 ++++++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 45 deletions(-)
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index 3919ab7..4161d20 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -40,9 +40,17 @@
#include "closestream.h"
#include "pathnames.h"
-static int quiet;
+struct mountpoint_control {
+ char *path;
+ dev_t dev;
+ struct stat st;
+ unsigned int
+ dev_devno:1,
+ fs_devno:1,
+ quiet:1;
+};
-static int dir_to_device(const char *spec, dev_t *dev)
+static int dir_to_device(struct mountpoint_control *ctl)
{
struct libmnt_table *tb = mnt_new_table_from_file(_PATH_PROC_MOUNTINFO);
struct libmnt_fs *fs;
@@ -54,16 +62,13 @@ static int dir_to_device(const char *spec, dev_t *dev)
* Fallback. Traditional way to detect mountpoints. This way
* is independent on /proc, but not able to detect bind mounts.
*/
- struct stat pst, st;
+ struct stat pst;
char buf[PATH_MAX], *cn;
int len;
- if (stat(spec, &st) != 0)
- return -1;
-
- cn = mnt_resolve_path(spec, NULL); /* canonicalize */
+ cn = mnt_resolve_path(ctl->path, NULL); /* canonicalize */
- len = snprintf(buf, sizeof(buf), "%s/..", cn ? cn : spec);
+ len = snprintf(buf, sizeof(buf), "%s/..", cn ? cn : ctl->path);
free(cn);
if (len < 0 || (size_t) len + 1 > sizeof(buf))
@@ -71,9 +76,9 @@ static int dir_to_device(const char *spec, dev_t *dev)
if (stat(buf, &pst) !=0)
return -1;
- if ((st.st_dev != pst.st_dev) ||
- (st.st_dev == pst.st_dev && st.st_ino == pst.st_ino)) {
- *dev = st.st_dev;
+ if ((ctl->st.st_dev != pst.st_dev) ||
+ (ctl->st.st_dev == pst.st_dev && ctl->st.st_ino == pst.st_ino)) {
+ ctl->dev = ctl->st.st_dev;
return 0;
}
@@ -85,9 +90,9 @@ static int dir_to_device(const char *spec, dev_t *dev)
mnt_table_set_cache(tb, cache);
mnt_unref_cache(cache);
- fs = mnt_table_find_target(tb, spec, MNT_ITER_BACKWARD);
+ fs = mnt_table_find_target(tb, ctl->path, MNT_ITER_BACKWARD);
if (fs && mnt_fs_get_target(fs)) {
- *dev = mnt_fs_get_devno(fs);
+ ctl->dev = mnt_fs_get_devno(fs);
rc = 0;
}
@@ -95,20 +100,14 @@ static int dir_to_device(const char *spec, dev_t *dev)
return rc;
}
-static int print_devno(const char *devname, struct stat *st)
+static int print_devno(const struct mountpoint_control *ctl)
{
- struct stat stbuf;
-
- if (!st && stat(devname, &stbuf) == 0)
- st = &stbuf;
- if (!st)
- return -1;
- if (!S_ISBLK(st->st_mode)) {
- if (!quiet)
- warnx(_("%s: not a block device"), devname);
+ if (!S_ISBLK(ctl->st.st_mode)) {
+ if (!ctl->quiet)
+ warnx(_("%s: not a block device"), ctl->path);
return -1;
}
- printf("%u:%u\n", major(st->st_rdev), minor(st->st_rdev));
+ printf("%u:%u\n", major(ctl->st.st_rdev), minor(ctl->st.st_rdev));
return 0;
}
@@ -133,9 +132,8 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
int main(int argc, char **argv)
{
- int c, fs_devno = 0, dev_devno = 0, rc = 0;
- char *spec;
- struct stat st;
+ int c, rc = 0;
+ struct mountpoint_control ctl = { 0 };
static const struct option longopts[] = {
{ "quiet", 0, 0, 'q' },
@@ -157,13 +155,13 @@ int main(int argc, char **argv)
switch(c) {
case 'q':
- quiet = 1;
+ ctl.quiet = 1;
break;
case 'd':
- fs_devno = 1;
+ ctl.fs_devno = 1;
break;
case 'x':
- dev_devno = 1;
+ ctl.dev_devno = 1;
break;
case 'h':
usage(stdout);
@@ -180,27 +178,25 @@ int main(int argc, char **argv)
if (optind + 1 != argc)
usage(stderr);
- spec = argv[optind++];
+ ctl.path = argv[optind];
- if (stat(spec, &st)) {
- if (!quiet)
- err(EXIT_FAILURE, "%s", spec);
+ if (stat(ctl.path, &ctl.st)) {
+ if (!ctl.quiet)
+ err(EXIT_FAILURE, "%s", ctl.path);
return EXIT_FAILURE;
}
- if (dev_devno)
- rc = print_devno(spec, &st);
+ if (ctl.dev_devno)
+ rc = print_devno(&ctl);
else {
- dev_t src;
-
- if ( dir_to_device(spec, &src)) {
- if (!quiet)
- printf(_("%s is not a mountpoint\n"), spec);
+ if (dir_to_device(&ctl)) {
+ if (!ctl.quiet)
+ printf(_("%s is not a mountpoint\n"), ctl.path);
return EXIT_FAILURE;
}
- if (fs_devno)
- printf("%u:%u\n", major(src), minor(src));
- else if (!quiet)
- printf(_("%s is a mountpoint\n"), spec);
+ if (ctl.fs_devno)
+ printf("%u:%u\n", major(ctl.dev), minor(ctl.dev));
+ else if (!ctl.quiet)
+ printf(_("%s is a mountpoint\n"), ctl.path);
}
return rc ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 10/17] mkfs.minix: fix couple compiler warnings
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (8 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 09/17] mountpoint: add struct mountpoint_control Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:42 ` [PATCH 11/17] mountpoint: simplify if statement Sami Kerola
` (8 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
disk-utils/mkfs.minix.c:366:3: warning: ISO C forbids 'return' with
expression, in function returning void [-Wpedantic]
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
disk-utils/mkfs.minix.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/disk-utils/mkfs.minix.c b/disk-utils/mkfs.minix.c
index 9fdfc63..2dafc86 100644
--- a/disk-utils/mkfs.minix.c
+++ b/disk-utils/mkfs.minix.c
@@ -362,9 +362,11 @@ static void make_bad_inode_v2_v3 (void)
static void make_bad_inode(void)
{
- if (fs_version < 2)
- return make_bad_inode_v1();
- return make_bad_inode_v2_v3();
+ if (fs_version < 2) {
+ make_bad_inode_v1();
+ return;
+ }
+ make_bad_inode_v2_v3();
}
static void make_root_inode_v1(void) {
@@ -412,9 +414,11 @@ static void make_root_inode_v2_v3 (void) {
static void make_root_inode(void)
{
- if (fs_version < 2)
- return make_root_inode_v1();
- return make_root_inode_v2_v3();
+ if (fs_version < 2) {
+ make_root_inode_v1();
+ return;
+ }
+ make_root_inode_v2_v3();
}
static void super_set_nzones(void)
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/17] mountpoint: simplify if statement
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (9 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 10/17] mkfs.minix: fix couple compiler warnings Sami Kerola
@ 2014-09-07 12:42 ` Sami Kerola
2014-09-07 12:43 ` [PATCH 12/17] eject: add struct eject_control to remove global variables Sami Kerola
` (7 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:42 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Returning straight after print_devno() makes the code to be more obvious
and removes need for long else statement.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/mountpoint.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/sys-utils/mountpoint.c b/sys-utils/mountpoint.c
index 3392cdd..3155dd4 100644
--- a/sys-utils/mountpoint.c
+++ b/sys-utils/mountpoint.c
@@ -132,7 +132,7 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
int main(int argc, char **argv)
{
- int c, rc = 0;
+ int c;
struct mountpoint_control ctl = { 0 };
static const struct option longopts[] = {
@@ -186,18 +186,15 @@ int main(int argc, char **argv)
return EXIT_FAILURE;
}
if (ctl.dev_devno)
- rc = print_devno(&ctl);
- else {
- if (dir_to_device(&ctl)) {
- if (!ctl.quiet)
- printf(_("%s is not a mountpoint\n"), ctl.devname);
- return EXIT_FAILURE;
- }
- if (ctl.fs_devno)
- printf("%u:%u\n", major(ctl.dev), minor(ctl.dev));
- else if (!ctl.quiet)
- printf(_("%s is a mountpoint\n"), ctl.devname);
+ return print_devno(&ctl) ? EXIT_FAILURE : EXIT_SUCCESS;
+ if (dir_to_device(&ctl)) {
+ if (!ctl.quiet)
+ printf(_("%s is not a mountpoint\n"), ctl.devname);
+ return EXIT_FAILURE;
}
-
- return rc ? EXIT_FAILURE : EXIT_SUCCESS;
+ if (ctl.fs_devno)
+ printf("%u:%u\n", major(ctl.dev), minor(ctl.dev));
+ else if (!ctl.quiet)
+ printf(_("%s is a mountpoint\n"), ctl.devname);
+ return EXIT_SUCCESS;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/17] eject: add struct eject_control to remove global variables
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (10 preceding siblings ...)
2014-09-07 12:42 ` [PATCH 11/17] mountpoint: simplify if statement Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-12 9:26 ` Karel Zak
2014-09-07 12:43 ` [PATCH 13/17] eject: make open_device() and select_speed() to use struct eject_control Sami Kerola
` (6 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/eject.c | 394 +++++++++++++++++++++++++++---------------------------
1 file changed, 198 insertions(+), 196 deletions(-)
diff --git a/sys-utils/eject.c b/sys-utils/eject.c
index 03744c7..767ede7 100644
--- a/sys-utils/eject.c
+++ b/sys-utils/eject.c
@@ -81,31 +81,34 @@ static const char * const hotplug_subsystems[] = {
"ccw"
};
-/* Global Variables */
-static int a_option; /* command flags and arguments */
-static int c_option;
-static int d_option;
-static int f_option;
-static int F_option;
-static int n_option;
-static int q_option;
-static int r_option;
-static int s_option;
-static int t_option;
-static int T_option;
-static int X_option;
-static int v_option;
-static int x_option;
-static int p_option;
-static int m_option;
-static int M_option;
-static int i_option;
-static int a_arg;
-static int i_arg;
-static long int c_arg;
-static long int x_arg;
-
-struct libmnt_table *mtab;
+struct eject_control {
+ struct libmnt_table *mtab;
+ char *device; /* device or mount point to be ejected */
+ int fd; /* file descriptor for device */
+ uint32_t /* command flags and arguments */
+ a_option:1,
+ c_option:1,
+ d_option:1,
+ F_option:1,
+ f_option:1,
+ i_option:1,
+ M_option:1,
+ m_option:1,
+ n_option:1,
+ p_option:1,
+ q_option:1,
+ r_option:1,
+ s_option:1,
+ T_option:1,
+ t_option:1,
+ v_option:1,
+ X_option:1,
+ x_option:1,
+ a_arg:1,
+ i_arg:1;
+ long int c_arg; /* changer slot number */
+ long int x_arg; /* cd speed */
+};
static void vinfo(const char *fmt, va_list va)
{
@@ -114,11 +117,11 @@ static void vinfo(const char *fmt, va_list va)
fputc('\n', stdout);
}
-static inline void verbose(const char *fmt, ...)
+static inline void verbose(const struct eject_control *ctl, const char *fmt, ...)
{
va_list va;
- if (!v_option)
+ if (!ctl->v_option)
return;
va_start(va, fmt);
@@ -174,7 +177,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
/* Handle command line options. */
-static void parse_args(int argc, char **argv, char **device)
+static void parse_args(struct eject_control *ctl, int argc, char **argv)
{
static const struct option long_opts[] =
{
@@ -206,75 +209,75 @@ static void parse_args(int argc, char **argv, char **device)
"a:c:i:x:dfFhnqrstTXvVpmM", long_opts, NULL)) != -1) {
switch (c) {
case 'a':
- a_option = 1;
+ ctl->a_option = 1;
if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
- a_arg = 0;
+ ctl->a_arg = 0;
else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
- a_arg = 1;
+ ctl->a_arg = 1;
else
errx(EXIT_FAILURE, _("invalid argument to --auto/-a option"));
break;
case 'c':
- c_option = 1;
- c_arg = strtoul_or_err(optarg, _("invalid argument to --changerslot/-c option"));
+ ctl->c_option = 1;
+ ctl->c_arg = strtoul_or_err(optarg, _("invalid argument to --changerslot/-c option"));
break;
case 'x':
- x_option = 1;
- x_arg = strtoul_or_err(optarg, _("invalid argument to --cdspeed/-x option"));
+ ctl->x_option = 1;
+ ctl->x_arg = strtoul_or_err(optarg, _("invalid argument to --cdspeed/-x option"));
break;
case 'd':
- d_option = 1;
+ ctl->d_option = 1;
break;
case 'f':
- f_option = 1;
+ ctl->f_option = 1;
break;
case 'F':
- F_option = 1;
+ ctl->F_option = 1;
break;
case 'h':
usage(stdout);
break;
case 'i':
- i_option = 1;
+ ctl->i_option = 1;
if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
- i_arg = 0;
+ ctl->i_arg = 0;
else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
- i_arg = 1;
+ ctl->i_arg = 1;
else
errx(EXIT_FAILURE, _("invalid argument to --manualeject/-i option"));
break;
case 'm':
- m_option = 1;
+ ctl->m_option = 1;
break;
case 'M':
- M_option = 1;
+ ctl->M_option = 1;
break;
case 'n':
- n_option = 1;
+ ctl->n_option = 1;
break;
case 'p':
- p_option = 1;
+ ctl->p_option = 1;
break;
case 'q':
- q_option = 1;
+ ctl->q_option = 1;
break;
case 'r':
- r_option = 1;
+ ctl->r_option = 1;
break;
case 's':
- s_option = 1;
+ ctl->s_option = 1;
break;
case 't':
- t_option = 1;
+ ctl->t_option = 1;
break;
case 'T':
- T_option = 1;
+ ctl->T_option = 1;
break;
case 'X':
- X_option = 1;
+ ctl->X_option = 1;
break;
case 'v':
- v_option = 1;
+ ctl->v_option = 1;
break;
case 'V':
printf(UTIL_LINUX_VERSION);
@@ -292,7 +295,7 @@ static void parse_args(int argc, char **argv, char **device)
errx(EXIT_FAILURE, _("too many arguments"));
if ((argc - optind) == 1)
- *device = xstrdup(argv[optind]);
+ ctl->device = xstrdup(argv[optind]);
}
/*
@@ -323,15 +326,15 @@ static char *find_device(const char *name)
}
/* Set or clear auto-eject mode. */
-static void auto_eject(int fd, int on)
+static void auto_eject(const struct eject_control *ctl)
{
int status = -1;
#if defined(CDROM_SET_OPTIONS) && defined(CDROM_CLEAR_OPTIONS)
- if (on)
- status = ioctl(fd, CDROM_SET_OPTIONS, CDO_AUTO_EJECT);
+ if (ctl->a_arg)
+ status = ioctl(ctl->fd, CDROM_SET_OPTIONS, CDO_AUTO_EJECT);
else
- status = ioctl(fd, CDROM_CLEAR_OPTIONS, CDO_AUTO_EJECT);
+ status = ioctl(ctl->fd, CDROM_CLEAR_OPTIONS, CDO_AUTO_EJECT);
#else
errno = ENOSYS;
#endif
@@ -350,12 +353,12 @@ static void auto_eject(int fd, int on)
* EBUSY Attempt to unlock when multiple users
* have the drive open and not CAP_SYS_ADMIN
*/
-static void manual_eject(int fd, int on)
+static void manual_eject(const struct eject_control *ctl)
{
- if (ioctl(fd, CDROM_LOCKDOOR, on) < 0)
+ if (ioctl(ctl->fd, CDROM_LOCKDOOR, ctl->i_arg) < 0)
err(EXIT_FAILURE, _("CD-ROM lock door command failed"));
- if (on)
+ if (ctl->i_arg)
info(_("CD-Drive may NOT be ejected with device button"));
else
info(_("CD-Drive may be ejected with device button"));
@@ -365,14 +368,14 @@ static void manual_eject(int fd, int on)
* Changer select. CDROM_SELECT_DISC is preferred, older kernels used
* CDROMLOADFROMSLOT.
*/
-static void changer_select(int fd, int slot)
+static void changer_select(const struct eject_control *ctl)
{
#ifdef CDROM_SELECT_DISC
- if (ioctl(fd, CDROM_SELECT_DISC, slot) < 0)
+ if (ioctl(ctl->fd, CDROM_SELECT_DISC, ctl->c_arg) < 0)
err(EXIT_FAILURE, _("CD-ROM select disc command failed"));
#elif defined CDROMLOADFROMSLOT
- if (ioctl(fd, CDROMLOADFROMSLOT, slot) != 0)
+ if (ioctl(ctl->fd, CDROMLOADFROMSLOT, ctl->c_arg) != 0)
err(EXIT_FAILURE, _("CD-ROM load from slot command failed"));
#else
warnx(_("IDE/ATAPI CD-ROM changer not supported by this kernel\n") );
@@ -557,18 +560,18 @@ static int read_speed(const char *devname)
/*
* List Speed of CD-ROM drive.
*/
-static void list_speeds(const char *name, int fd)
+static void list_speeds(const struct eject_control *ctl)
{
#ifdef CDROM_SELECT_SPEED
int max_speed, curr_speed = 0, prev_speed;
- select_speed(fd, 0);
- max_speed = read_speed(name);
+ select_speed(ctl->fd, 0);
+ max_speed = read_speed(ctl->device);
while (curr_speed < max_speed) {
prev_speed = curr_speed;
- select_speed(fd, prev_speed + 1);
- curr_speed = read_speed(name);
+ select_speed(ctl->fd, prev_speed + 1);
+ curr_speed = read_speed(ctl->device);
if (curr_speed > prev_speed)
printf("%d ", curr_speed);
else
@@ -584,7 +587,7 @@ static void list_speeds(const char *name, int fd)
/*
* Eject using SCSI SG_IO commands. Return 1 if successful, 0 otherwise.
*/
-static int eject_scsi(int fd)
+static int eject_scsi(const struct eject_control *ctl)
{
int status, k;
sg_io_hdr_t io_hdr;
@@ -594,8 +597,8 @@ static int eject_scsi(int fd)
unsigned char inqBuff[2];
unsigned char sense_buffer[32];
- if ((ioctl(fd, SG_GET_VERSION_NUM, &k) < 0) || (k < 30000)) {
- verbose(_("not an sg device, or old sg driver"));
+ if ((ioctl(ctl->fd, SG_GET_VERSION_NUM, &k) < 0) || (k < 30000)) {
+ verbose(ctl, _("not an sg device, or old sg driver"));
return 0;
}
@@ -610,12 +613,12 @@ static int eject_scsi(int fd)
io_hdr.timeout = 10000;
io_hdr.cmdp = allowRmBlk;
- status = ioctl(fd, SG_IO, (void *)&io_hdr);
+ status = ioctl(ctl->fd, SG_IO, (void *)&io_hdr);
if (status < 0 || io_hdr.host_status || io_hdr.driver_status)
return 0;
io_hdr.cmdp = startStop1Blk;
- status = ioctl(fd, SG_IO, (void *)&io_hdr);
+ status = ioctl(ctl->fd, SG_IO, (void *)&io_hdr);
if (status < 0 || io_hdr.host_status)
return 0;
@@ -630,12 +633,12 @@ static int eject_scsi(int fd)
return 0;
io_hdr.cmdp = startStop2Blk;
- status = ioctl(fd, SG_IO, (void *)&io_hdr);
+ status = ioctl(ctl->fd, SG_IO, (void *)&io_hdr);
if (status < 0 || io_hdr.host_status || io_hdr.driver_status)
return 0;
/* force kernel to reread partition table when new disc inserted */
- ioctl(fd, BLKRRPART);
+ ioctl(ctl->fd, BLKRRPART);
return 1;
}
@@ -660,14 +663,14 @@ static int eject_tape(int fd)
/* umount a device. */
-static void umount_one(const char *name)
+static void umount_one(const struct eject_control *ctl, const char *name)
{
int status;
if (!name)
return;
- verbose(_("%s: unmounting"), name);
+ verbose(ctl, _("%s: unmounting"), name);
switch (fork()) {
case 0: /* child */
@@ -677,7 +680,7 @@ static void umount_one(const char *name)
if (setuid(getuid()) < 0)
err(EXIT_FAILURE, _("cannot set user id"));
- if (p_option)
+ if (ctl->p_option)
execl("/bin/umount", "/bin/umount", name, "-n", NULL);
else
execl("/bin/umount", "/bin/umount", name, NULL);
@@ -716,36 +719,36 @@ static int open_device(const char *name)
* See if device has been mounted by looking in mount table. If so, set
* device name and mount point name, and return 1, otherwise return 0.
*/
-static int device_get_mountpoint(char **devname, char **mnt)
+static int device_get_mountpoint(struct eject_control *ctl, char **devname, char **mnt)
{
struct libmnt_fs *fs;
int rc;
*mnt = NULL;
- if (!mtab) {
+ if (!ctl->mtab) {
struct libmnt_cache *cache;
- mtab = mnt_new_table();
- if (!mtab)
+ ctl->mtab = mnt_new_table();
+ if (!ctl->mtab)
err(EXIT_FAILURE, _("failed to initialize libmount table"));
cache = mnt_new_cache();
- mnt_table_set_cache(mtab, cache);
+ mnt_table_set_cache(ctl->mtab, cache);
mnt_unref_cache(cache);
- if (p_option)
- rc = mnt_table_parse_file(mtab, _PATH_PROC_MOUNTINFO);
+ if (ctl->p_option)
+ rc = mnt_table_parse_file(ctl->mtab, _PATH_PROC_MOUNTINFO);
else
- rc = mnt_table_parse_mtab(mtab, NULL);
+ rc = mnt_table_parse_mtab(ctl->mtab, NULL);
if (rc)
err(EXIT_FAILURE, _("failed to parse mount table"));
}
- fs = mnt_table_find_source(mtab, *devname, MNT_ITER_BACKWARD);
+ fs = mnt_table_find_source(ctl->mtab, *devname, MNT_ITER_BACKWARD);
if (!fs) {
/* maybe 'devname' is mountpoint rather than a real device */
- fs = mnt_table_find_target(mtab, *devname, MNT_ITER_BACKWARD);
+ fs = mnt_table_find_target(ctl->mtab, *devname, MNT_ITER_BACKWARD);
if (fs) {
free(*devname);
*devname = xstrdup(mnt_fs_get_source(fs));
@@ -774,7 +777,7 @@ static char *get_disk_devname(const char *device)
return st.st_rdev == diskno ? NULL : find_device(diskname);
}
-static int umount_partitions(const char *disk, int checkonly)
+static int umount_partitions(struct eject_control *ctl)
{
struct sysfs_cxt cxt = UL_SYSFSCXT_EMPTY;
dev_t devno;
@@ -782,7 +785,7 @@ static int umount_partitions(const char *disk, int checkonly)
struct dirent *d;
int count = 0;
- devno = sysfs_devname_to_devno(disk, NULL);
+ devno = sysfs_devname_to_devno(ctl->device, NULL);
if (sysfs_init(&cxt, devno, NULL) != 0)
return 0;
@@ -795,14 +798,14 @@ static int umount_partitions(const char *disk, int checkonly)
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
- if (sysfs_is_partition_dirent(dir, d, disk)) {
+ if (sysfs_is_partition_dirent(dir, d, ctl->device)) {
char *mnt = NULL;
char *dev = find_device(d->d_name);
- if (dev && device_get_mountpoint(&dev, &mnt) == 0) {
- verbose(_("%s: mounted on %s"), dev, mnt);
- if (!checkonly)
- umount_one(mnt);
+ if (dev && device_get_mountpoint(ctl, &dev, &mnt) == 0) {
+ verbose(ctl, _("%s: mounted on %s"), dev, mnt);
+ if (!ctl->M_option)
+ umount_one(ctl, mnt);
count++;
}
free(dev);
@@ -881,7 +884,7 @@ static char *get_subsystem(char *chain, char *buf, size_t bufsz)
return NULL;
}
-static int is_hotpluggable(const char* device)
+static int is_hotpluggable(const struct eject_control *ctl)
{
struct sysfs_cxt cxt = UL_SYSFSCXT_EMPTY;
char devchain[PATH_MAX];
@@ -891,13 +894,13 @@ static int is_hotpluggable(const char* device)
ssize_t sz;
char *sub;
- devno = sysfs_devname_to_devno(device, NULL);
+ devno = sysfs_devname_to_devno(ctl->device, NULL);
if (sysfs_init(&cxt, devno, NULL) != 0)
return 0;
/* check /sys/dev/block/<maj>:<min>/removable attribute */
if (sysfs_read_int(&cxt, "removable", &rc) == 0 && rc == 1) {
- verbose(_("%s: is removable device"), device);
+ verbose(ctl, _("%s: is removable device"), ctl->device);
goto done;
}
@@ -916,8 +919,8 @@ static int is_hotpluggable(const char* device)
while ((sub = get_subsystem(devchain, subbuf, sizeof(subbuf)))) {
rc = is_hotpluggable_subsystem(sub);
if (rc) {
- verbose(_("%s: connected by hotplug subsystem: %s"),
- device, sub);
+ verbose(ctl, _("%s: connected by hotplug subsystem: %s"),
+ ctl->device, sub);
break;
}
}
@@ -929,20 +932,20 @@ done:
/* handle -x option */
-static void set_device_speed(char *name)
+static void set_device_speed(const struct eject_control *ctl)
{
int fd;
- if (!x_option)
+ if (!ctl->x_option)
return;
- if (x_arg == 0)
- verbose(_("setting CD-ROM speed to auto"));
+ if (ctl->x_arg == 0)
+ verbose(ctl, _("setting CD-ROM speed to auto"));
else
- verbose(_("setting CD-ROM speed to %ldX"), x_arg);
+ verbose(ctl, _("setting CD-ROM speed to %ldX"), ctl->x_arg);
- fd = open_device(name);
- select_speed(fd, x_arg);
+ fd = open_device(ctl->device);
+ select_speed(fd, ctl->x_arg);
exit(EXIT_SUCCESS);
}
@@ -950,11 +953,10 @@ static void set_device_speed(char *name)
/* main program */
int main(int argc, char **argv)
{
- char *device = NULL;
char *disk = NULL;
char *mountpoint = NULL;
int worked = 0; /* set to 1 when successfully ejected */
- int fd; /* file descriptor for device */
+ struct eject_control ctl = { 0 };
setlocale(LC_ALL,"");
bindtextdomain(PACKAGE, LOCALEDIR);
@@ -962,118 +964,118 @@ int main(int argc, char **argv)
atexit(close_stdout);
/* parse the command line arguments */
- parse_args(argc, argv, &device);
+ parse_args(&ctl, argc, argv);
/* handle -d option */
- if (d_option) {
+ if (ctl.d_option) {
info(_("default device: `%s'"), EJECT_DEFAULT_DEVICE);
return EXIT_SUCCESS;
}
- if (!device) {
- device = mnt_resolve_path(EJECT_DEFAULT_DEVICE, NULL);
- verbose(_("using default device `%s'"), device);
+ if (!ctl.device) {
+ ctl.device = mnt_resolve_path(EJECT_DEFAULT_DEVICE, NULL);
+ verbose(&ctl, _("using default device `%s'"), ctl.device);
} else {
char *p;
- if (device[strlen(device)-1] == '/')
- device[strlen(device)-1] = '\0';
+ if (ctl.device[strlen(ctl.device) - 1] == '/')
+ ctl.device[strlen(ctl.device) - 1] = '\0';
/* figure out full device or mount point name */
- p = find_device(device);
+ p = find_device(ctl.device);
if (p)
- free(device);
+ free(ctl.device);
else
- p = device;
+ p = ctl.device;
- device = mnt_resolve_spec(p, NULL);
+ ctl.device = mnt_resolve_spec(p, NULL);
free(p);
}
- if (!device)
- errx(EXIT_FAILURE, _("%s: unable to find device"), device);
+ if (!ctl.device)
+ errx(EXIT_FAILURE, _("%s: unable to find device"), ctl.device);
- verbose(_("device name is `%s'"), device);
+ verbose(&ctl, _("device name is `%s'"), ctl.device);
- device_get_mountpoint(&device, &mountpoint);
+ device_get_mountpoint(&ctl, &ctl.device, &mountpoint);
if (mountpoint)
- verbose(_("%s: mounted on %s"), device, mountpoint);
+ verbose(&ctl, _("%s: mounted on %s"), ctl.device, mountpoint);
else
- verbose(_("%s: not mounted"), device);
+ verbose(&ctl, _("%s: not mounted"), ctl.device);
- disk = get_disk_devname(device);
+ disk = get_disk_devname(ctl.device);
if (disk) {
- verbose(_("%s: disc device: %s (disk device will be used for eject)"), device, disk);
- free(device);
- device = disk;
+ verbose(&ctl, _("%s: disc device: %s (disk device will be used for eject)"), ctl.device, disk);
+ free(ctl.device);
+ ctl.device = disk;
disk = NULL;
} else {
struct stat st;
- if (stat(device, &st) != 0 || !S_ISBLK(st.st_mode))
+ if (stat(ctl.device, &st) != 0 || !S_ISBLK(st.st_mode))
errx(EXIT_FAILURE, _("%s: not found mountpoint or device "
- "with the given name"), device);
+ "with the given name"), ctl.device);
- verbose(_("%s: is whole-disk device"), device);
+ verbose(&ctl, _("%s: is whole-disk device"), ctl.device);
}
- if (F_option == 0 && is_hotpluggable(device) == 0)
- errx(EXIT_FAILURE, _("%s: is not hot-pluggable device"), device);
+ if (ctl.F_option == 0 && is_hotpluggable(&ctl) == 0)
+ errx(EXIT_FAILURE, _("%s: is not hot-pluggable device"), ctl.device);
/* handle -n option */
- if (n_option) {
- info(_("device is `%s'"), device);
- verbose(_("exiting due to -n/--noop option"));
+ if (ctl.n_option) {
+ info(_("device is `%s'"), ctl.device);
+ verbose(&ctl, _("exiting due to -n/--noop option"));
return EXIT_SUCCESS;
}
/* handle -i option */
- if (i_option) {
- fd = open_device(device);
- manual_eject(fd, i_arg);
+ if (ctl.i_option) {
+ ctl.fd = open_device(ctl.device);
+ manual_eject(&ctl);
return EXIT_SUCCESS;
}
/* handle -a option */
- if (a_option) {
- if (a_arg)
- verbose(_("%s: enabling auto-eject mode"), device);
+ if (ctl.a_option) {
+ if (ctl.a_arg)
+ verbose(&ctl, _("%s: enabling auto-eject mode"), ctl.device);
else
- verbose(_("%s: disabling auto-eject mode"), device);
- fd = open_device(device);
- auto_eject(fd, a_arg);
+ verbose(&ctl, _("%s: disabling auto-eject mode"), ctl.device);
+ ctl.fd = open_device(ctl.device);
+ auto_eject(&ctl);
return EXIT_SUCCESS;
}
/* handle -t option */
- if (t_option) {
- verbose(_("%s: closing tray"), device);
- fd = open_device(device);
- close_tray(fd);
- set_device_speed(device);
+ if (ctl.t_option) {
+ verbose(&ctl, _("%s: closing tray"), ctl.device);
+ ctl.fd = open_device(ctl.device);
+ close_tray(ctl.fd);
+ set_device_speed(&ctl);
return EXIT_SUCCESS;
}
/* handle -T option */
- if (T_option) {
- verbose(_("%s: toggling tray"), device);
- fd = open_device(device);
- toggle_tray(fd);
- set_device_speed(device);
+ if (ctl.T_option) {
+ verbose(&ctl, _("%s: toggling tray"), ctl.device);
+ ctl.fd = open_device(ctl.device);
+ toggle_tray(ctl.fd);
+ set_device_speed(&ctl);
return EXIT_SUCCESS;
}
/* handle -X option */
- if (X_option) {
- verbose(_("%s: listing CD-ROM speed"), device);
- fd = open_device(device);
- list_speeds(device, fd);
+ if (ctl.X_option) {
+ verbose(&ctl, _("%s: listing CD-ROM speed"), ctl.device);
+ ctl.fd = open_device(ctl.device);
+ list_speeds(&ctl);
return EXIT_SUCCESS;
}
/* handle -x option only */
- if (!c_option)
- set_device_speed(device);
+ if (!ctl.c_option)
+ set_device_speed(&ctl);
/*
@@ -1081,62 +1083,62 @@ int main(int argc, char **argv)
* mountpoint if -M is specified, otherwise print error of another
* partition is mounted.
*/
- if (!m_option) {
- int ct = umount_partitions(device, M_option);
+ if (!ctl.m_option) {
+ int ct = umount_partitions(&ctl);
if (ct == 0 && mountpoint)
- umount_one(mountpoint); /* probably whole-device */
+ umount_one(&ctl, mountpoint); /* probably whole-device */
- if (M_option) {
+ if (ctl.M_option) {
if (ct == 1 && mountpoint)
- umount_one(mountpoint);
+ umount_one(&ctl, mountpoint);
else if (ct)
- errx(EXIT_FAILURE, _("error: %s: device in use"), device);
+ errx(EXIT_FAILURE, _("error: %s: device in use"), ctl.device);
}
}
/* handle -c option */
- if (c_option) {
- verbose(_("%s: selecting CD-ROM disc #%ld"), device, c_arg);
- fd = open_device(device);
- changer_select(fd, c_arg);
- set_device_speed(device);
+ if (ctl.c_option) {
+ verbose(&ctl, _("%s: selecting CD-ROM disc #%ld"), ctl.device, ctl.c_arg);
+ ctl.fd = open_device(ctl.device);
+ changer_select(&ctl);
+ set_device_speed(&ctl);
return EXIT_SUCCESS;
}
/* if user did not specify type of eject, try all four methods */
- if (r_option + s_option + f_option + q_option == 0)
- r_option = s_option = f_option = q_option = 1;
+ if (ctl.r_option + ctl.s_option + ctl.f_option + ctl.q_option == 0)
+ ctl.r_option = ctl.s_option = ctl.f_option = ctl.q_option = 1;
/* open device */
- fd = open_device(device);
+ ctl.fd = open_device(ctl.device);
/* try various methods of ejecting until it works */
- if (r_option) {
- verbose(_("%s: trying to eject using CD-ROM eject command"), device);
- worked = eject_cdrom(fd);
- verbose(worked ? _("CD-ROM eject command succeeded") :
+ if (ctl.r_option) {
+ verbose(&ctl, _("%s: trying to eject using CD-ROM eject command"), ctl.device);
+ worked = eject_cdrom(ctl.fd);
+ verbose(&ctl, worked ? _("CD-ROM eject command succeeded") :
_("CD-ROM eject command failed"));
}
- if (s_option && !worked) {
- verbose(_("%s: trying to eject using SCSI commands"), device);
- worked = eject_scsi(fd);
- verbose(worked ? _("SCSI eject succeeded") :
+ if (ctl.s_option && !worked) {
+ verbose(&ctl, _("%s: trying to eject using SCSI commands"), ctl.device);
+ worked = eject_scsi(&ctl);
+ verbose(&ctl, worked ? _("SCSI eject succeeded") :
_("SCSI eject failed"));
}
- if (f_option && !worked) {
- verbose(_("%s: trying to eject using floppy eject command"), device);
- worked = eject_floppy(fd);
- verbose(worked ? _("floppy eject command succeeded") :
+ if (ctl.f_option && !worked) {
+ verbose(&ctl, _("%s: trying to eject using floppy eject command"), ctl.device);
+ worked = eject_floppy(ctl.fd);
+ verbose(&ctl, worked ? _("floppy eject command succeeded") :
_("floppy eject command failed"));
}
- if (q_option && !worked) {
- verbose(_("%s: trying to eject using tape offline command"), device);
- worked = eject_tape(fd);
- verbose(worked ? _("tape offline command succeeded") :
+ if (ctl.q_option && !worked) {
+ verbose(&ctl, _("%s: trying to eject using tape offline command"), ctl.device);
+ worked = eject_tape(ctl.fd);
+ verbose(&ctl, worked ? _("tape offline command succeeded") :
_("tape offline command failed"));
}
@@ -1144,11 +1146,11 @@ int main(int argc, char **argv)
errx(EXIT_FAILURE, _("unable to eject"));
/* cleanup */
- close(fd);
- free(device);
+ close(ctl.fd);
+ free(ctl.device);
free(mountpoint);
- mnt_unref_table(mtab);
+ mnt_unref_table(ctl.mtab);
return EXIT_SUCCESS;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 12/17] eject: add struct eject_control to remove global variables
2014-09-07 12:43 ` [PATCH 12/17] eject: add struct eject_control to remove global variables Sami Kerola
@ 2014-09-12 9:26 ` Karel Zak
2014-09-16 20:39 ` Sami Kerola
0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2014-09-12 9:26 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Sep 07, 2014 at 01:43:00PM +0100, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> sys-utils/eject.c | 394 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 198 insertions(+), 196 deletions(-)
>
> diff --git a/sys-utils/eject.c b/sys-utils/eject.c
> index 03744c7..767ede7 100644
> --- a/sys-utils/eject.c
> +++ b/sys-utils/eject.c
> @@ -81,31 +81,34 @@ static const char * const hotplug_subsystems[] = {
> "ccw"
> };
>
> -/* Global Variables */
> -static int a_option; /* command flags and arguments */
> -static int c_option;
> -static int d_option;
> -static int f_option;
> -static int F_option;
> -static int n_option;
> -static int q_option;
> -static int r_option;
> -static int s_option;
> -static int t_option;
> -static int T_option;
> -static int X_option;
> -static int v_option;
> -static int x_option;
> -static int p_option;
> -static int m_option;
> -static int M_option;
> -static int i_option;
> -static int a_arg;
> -static int i_arg;
> -static long int c_arg;
> -static long int x_arg;
> -
> -struct libmnt_table *mtab;
> +struct eject_control {
> + struct libmnt_table *mtab;
> + char *device; /* device or mount point to be ejected */
> + int fd; /* file descriptor for device */
> + uint32_t /* command flags and arguments */
Again, don't be creative, use "unsigned int". The reality is that
you have zero control on the way how compiler pack the bits.
C99:
An implementation may allocate any addressable storage unit large
enough to hold a bit- field. If enough space remains, a bit-field that
immediately follows another bit-field in a structure shall be packed
into adjacent bits of the same unit. If insufficient space remains,
whether a bit-field that does not fit is put into the next unit or
overlaps adjacent units is implementation-defined. The order of
allocation of bit-fields within a unit (high-order to low-order or
low-order to high-order) is implementation-defined. The alignment of
the addressable storage unit is unspecified.
We usually use uintXX_t for on-disk data, to be sure with
large numbers (uintmax_t) or for things that have any dependence on
system. It's really unnecessary for any random control struct.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 12/17] eject: add struct eject_control to remove global variables
2014-09-12 9:26 ` Karel Zak
@ 2014-09-16 20:39 ` Sami Kerola
0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-16 20:39 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sun, Sep 07, 2014 at 01:43:00PM +0100, Sami Kerola wrote:
>> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
>> ---
>> sys-utils/eject.c | 394 +++++++++++++++++++++++++++---------------------------
>> 1 file changed, 198 insertions(+), 196 deletions(-)
>>
>> diff --git a/sys-utils/eject.c b/sys-utils/eject.c
>> index 03744c7..767ede7 100644
>> --- a/sys-utils/eject.c
>> +++ b/sys-utils/eject.c
>> @@ -81,31 +81,34 @@ static const char * const hotplug_subsystems[] = {
>> "ccw"
>> };
>>
>> -/* Global Variables */
>> -static int a_option; /* command flags and arguments */
>> -static int c_option;
>> -static int d_option;
>> -static int f_option;
>> -static int F_option;
>> -static int n_option;
>> -static int q_option;
>> -static int r_option;
>> -static int s_option;
>> -static int t_option;
>> -static int T_option;
>> -static int X_option;
>> -static int v_option;
>> -static int x_option;
>> -static int p_option;
>> -static int m_option;
>> -static int M_option;
>> -static int i_option;
>> -static int a_arg;
>> -static int i_arg;
>> -static long int c_arg;
>> -static long int x_arg;
>> -
>> -struct libmnt_table *mtab;
>> +struct eject_control {
>> + struct libmnt_table *mtab;
>> + char *device; /* device or mount point to be ejected */
>> + int fd; /* file descriptor for device */
>> + uint32_t /* command flags and arguments */
>
> Again, don't be creative, use "unsigned int". The reality is that
> you have zero control on the way how compiler pack the bits.
Corrected version is easiest to get from my github branch misc. I will
send update to pull request shortly.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 13/17] eject: make open_device() and select_speed() to use struct eject_control
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (11 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 12/17] eject: add struct eject_control to remove global variables Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-07 12:43 ` [PATCH 14/17] hwclock: remove referal to deprecated keyboard interface Sami Kerola
` (5 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/eject.c | 54 +++++++++++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/sys-utils/eject.c b/sys-utils/eject.c
index 767ede7..12a9bbb 100644
--- a/sys-utils/eject.c
+++ b/sys-utils/eject.c
@@ -488,10 +488,10 @@ static void toggle_tray(int fd)
* Thanks to Roland Krivanek (krivanek@fmph.uniba.sk)
* http://dmpc.dbp.fmph.uniba.sk/~krivanek/cdrom_speed/
*/
-static void select_speed(int fd, int speed)
+static void select_speed(const struct eject_control *ctl)
{
#ifdef CDROM_SELECT_SPEED
- if (ioctl(fd, CDROM_SELECT_SPEED, speed) != 0)
+ if (ioctl(ctl->fd, CDROM_SELECT_SPEED, ctl->x_arg) != 0)
err(EXIT_FAILURE, _("CD-ROM select speed command failed"));
#else
warnx(_("CD-ROM select speed command not supported by this kernel"));
@@ -560,22 +560,22 @@ static int read_speed(const char *devname)
/*
* List Speed of CD-ROM drive.
*/
-static void list_speeds(const struct eject_control *ctl)
+static void list_speeds(struct eject_control *ctl)
{
#ifdef CDROM_SELECT_SPEED
- int max_speed, curr_speed = 0, prev_speed;
+ int max_speed, curr_speed = 0;
- select_speed(ctl->fd, 0);
+ select_speed(ctl);
max_speed = read_speed(ctl->device);
while (curr_speed < max_speed) {
- prev_speed = curr_speed;
- select_speed(ctl->fd, prev_speed + 1);
+ ctl->x_arg = curr_speed + 1;
+ select_speed(ctl);
curr_speed = read_speed(ctl->device);
- if (curr_speed > prev_speed)
+ if (ctl->x_arg < curr_speed)
printf("%d ", curr_speed);
else
- curr_speed = prev_speed + 1;
+ curr_speed = ctl->x_arg + 1;
}
printf("\n");
@@ -704,15 +704,13 @@ static void umount_one(const struct eject_control *ctl, const char *name)
}
/* Open a device file. */
-static int open_device(const char *name)
+static void open_device(struct eject_control *ctl)
{
- int fd = open(name, O_RDWR|O_NONBLOCK);
-
- if (fd < 0)
- fd = open(name, O_RDONLY|O_NONBLOCK);
- if (fd == -1)
- err(EXIT_FAILURE, _("cannot open %s"), name);
- return fd;
+ ctl->fd = open(ctl->device, O_RDWR | O_NONBLOCK);
+ if (ctl->fd < 0)
+ ctl->fd = open(ctl->device, O_RDONLY | O_NONBLOCK);
+ if (ctl->fd == -1)
+ err(EXIT_FAILURE, _("cannot open %s"), ctl->device);
}
/*
@@ -932,10 +930,8 @@ done:
/* handle -x option */
-static void set_device_speed(const struct eject_control *ctl)
+static void set_device_speed(struct eject_control *ctl)
{
- int fd;
-
if (!ctl->x_option)
return;
@@ -944,8 +940,8 @@ static void set_device_speed(const struct eject_control *ctl)
else
verbose(ctl, _("setting CD-ROM speed to %ldX"), ctl->x_arg);
- fd = open_device(ctl->device);
- select_speed(fd, ctl->x_arg);
+ open_device(ctl);
+ select_speed(ctl);
exit(EXIT_SUCCESS);
}
@@ -1031,7 +1027,7 @@ int main(int argc, char **argv)
/* handle -i option */
if (ctl.i_option) {
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
manual_eject(&ctl);
return EXIT_SUCCESS;
}
@@ -1042,7 +1038,7 @@ int main(int argc, char **argv)
verbose(&ctl, _("%s: enabling auto-eject mode"), ctl.device);
else
verbose(&ctl, _("%s: disabling auto-eject mode"), ctl.device);
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
auto_eject(&ctl);
return EXIT_SUCCESS;
}
@@ -1050,7 +1046,7 @@ int main(int argc, char **argv)
/* handle -t option */
if (ctl.t_option) {
verbose(&ctl, _("%s: closing tray"), ctl.device);
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
close_tray(ctl.fd);
set_device_speed(&ctl);
return EXIT_SUCCESS;
@@ -1059,7 +1055,7 @@ int main(int argc, char **argv)
/* handle -T option */
if (ctl.T_option) {
verbose(&ctl, _("%s: toggling tray"), ctl.device);
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
toggle_tray(ctl.fd);
set_device_speed(&ctl);
return EXIT_SUCCESS;
@@ -1068,7 +1064,7 @@ int main(int argc, char **argv)
/* handle -X option */
if (ctl.X_option) {
verbose(&ctl, _("%s: listing CD-ROM speed"), ctl.device);
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
list_speeds(&ctl);
return EXIT_SUCCESS;
}
@@ -1100,7 +1096,7 @@ int main(int argc, char **argv)
/* handle -c option */
if (ctl.c_option) {
verbose(&ctl, _("%s: selecting CD-ROM disc #%ld"), ctl.device, ctl.c_arg);
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
changer_select(&ctl);
set_device_speed(&ctl);
return EXIT_SUCCESS;
@@ -1111,7 +1107,7 @@ int main(int argc, char **argv)
ctl.r_option = ctl.s_option = ctl.f_option = ctl.q_option = 1;
/* open device */
- ctl.fd = open_device(ctl.device);
+ open_device(&ctl);
/* try various methods of ejecting until it works */
if (ctl.r_option) {
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 14/17] hwclock: remove referal to deprecated keyboard interface
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (12 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 13/17] eject: make open_device() and select_speed() to use struct eject_control Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-07 12:43 ` [PATCH 15/17] setarch: reindent code Sami Kerola
` (4 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
The KDGHWCLK has been gone quite a while.
Reference: http://marc.info/?l=linux-kernel&m=104171103925897
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/Makemodule.am | 3 +-
sys-utils/hwclock-kd.c | 180 ------------------------------------------------
sys-utils/hwclock.c | 3 -
sys-utils/hwclock.h | 1 -
4 files changed, 1 insertion(+), 186 deletions(-)
delete mode 100644 sys-utils/hwclock-kd.c
diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index 058bb62..2ae039c 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -353,8 +353,7 @@ PATHFILES += sys-utils/hwclock.8
hwclock_SOURCES = \
sys-utils/hwclock.c \
sys-utils/hwclock.h \
- sys-utils/hwclock-cmos.c \
- sys-utils/hwclock-kd.c
+ sys-utils/hwclock-cmos.c
if LINUX
hwclock_SOURCES += sys-utils/hwclock-rtc.c
endif
diff --git a/sys-utils/hwclock-kd.c b/sys-utils/hwclock-kd.c
deleted file mode 100644
index 3dac707..0000000
--- a/sys-utils/hwclock-kd.c
+++ /dev/null
@@ -1,180 +0,0 @@
-/*
- * kd.c - KDGHWCLK stuff, possibly m68k only, likely to be deprecated
- */
-#include "hwclock.h"
-
-#ifdef __m68k__
-
-# include <fcntl.h>
-# include <sysexits.h>
-# include <sys/ioctl.h>
-# include <unistd.h>
-
-# include "nls.h"
-
-/* Get defines for KDGHWCLK and KDSHWCLK (m68k) */
-# include <linux/kd.h>
-
-/* Even on m68k, if KDGHWCLK (antique) is not defined, don't build this */
-
-#endif
-
-#if !defined(__m68k__) || !defined(KDGHWCLK)
-
-#include <stddef.h>
-struct clock_ops *probe_for_kd_clock(void)
-{
- return NULL;
-}
-
-#else /* __m68k__ && KDGHWCLK */
-
-/* Opened by probe_for_kd_clock(), and never closed. */
-static int con_fd = -1;
-static char *con_fd_filename; /* usually "/dev/tty1" */
-
-/*
- * Wait for the top of a clock tick by calling KDGHWCLK in a busy loop until
- * we see it.
- */
-static int synchronize_to_clock_tick_kd(void)
-{
- /* The time when we were called (and started waiting) */
- struct hwclk_time start_time, nowtime;
- struct timeval begin, now;
-
- if (debug)
- printf(_("Waiting in loop for time from KDGHWCLK to change\n"));
-
- if (ioctl(con_fd, KDGHWCLK, &start_time) == -1) {
- warn(_("KDGHWCLK ioctl to read time failed"));
- return 3;
- }
-
- /*
- * Wait for change. Should be within a second, but in case something
- * weird happens, we have a time limit (1.5s) on this loop to reduce
- * the impact of this failure.
- */
- gettimeofday(&begin, NULL);
- do {
- /*
- * Added by Roman Hodek <Roman.Hodek@informatik.uni-erlangen.de>
- *
- * "The culprit is the fast loop with KDGHWCLK ioctls. It
- * seems the kernel gets confused by those on Amigas with
- * A2000 RTCs and simply hangs after some time. Inserting a
- * sleep helps."
- */
- xusleep(1);
-
- if (ioctl(con_fd, KDGHWCLK, &nowtime) == -1) {
- warn(_("KDGHWCLK ioctl to read time failed in loop"));
- return 3;
- }
- if (start_time.tm_sec != nowtime.tm_sec)
- break;
- gettimeofday(&now, NULL);
- if (time_diff(now, begin) > 1.5) {
- warnx(_("Timed out waiting for time change."));
- return 2;
- }
- } while (1);
-
- return 0;
-}
-
-/*
- * Read the hardware clock and return the current time via <tm> argument.
- * Use ioctls to /dev/tty1 on what we assume is an m68k machine.
- *
- * Note that we don't use /dev/console here. That might be a serial console.
- */
-static int read_hardware_clock_kd(struct tm *tm)
-{
- struct hwclk_time t;
-
- if (ioctl(con_fd, KDGHWCLK, &t) == -1) {
- warn(_("ioctl() failed to read time from %s"),
- con_fd_filename);
- hwclock_exit(EX_IOERR);
- }
-
- tm->tm_sec = t.sec;
- tm->tm_min = t.min;
- tm->tm_hour = t.hour;
- tm->tm_mday = t.day;
- tm->tm_mon = t.mon;
- tm->tm_year = t.year;
- tm->tm_wday = t.wday;
- tm->tm_isdst = -1; /* Don't know if it's Daylight Savings Time */
-
- return 0;
-}
-
-/*
- * Set the Hardware Clock to the time <new_broken_time>. Use ioctls to
- * /dev/tty1 on what we assume is an m68k machine.
- *
- * Note that we don't use /dev/console here. That might be a serial console.
- */
-static int set_hardware_clock_kd(const struct tm *new_broken_time)
-{
- struct hwclk_time t;
-
- t.sec = new_broken_time->tm_sec;
- t.min = new_broken_time->tm_min;
- t.hour = new_broken_time->tm_hour;
- t.day = new_broken_time->tm_mday;
- t.mon = new_broken_time->tm_mon;
- t.year = new_broken_time->tm_year;
- t.wday = new_broken_time->tm_wday;
-
- if (ioctl(con_fd, KDSHWCLK, &t) == -1) {
- warn(_("ioctl KDSHWCLK failed"));
- hwclock_exit(EX_IOERR);
- }
- return 0;
-}
-
-static int get_permissions_kd(void)
-{
- return 0;
-}
-
-static struct clock_ops kd = {
- N_("Using the KDGHWCLK interface to m68k clock."),
- get_permissions_kd,
- read_hardware_clock_kd,
- set_hardware_clock_kd,
- synchronize_to_clock_tick_kd,
-};
-
-/* return &kd if KDGHWCLK works, NULL otherwise */
-struct clock_ops *probe_for_kd_clock()
-{
- struct clock_ops *ret = NULL;
- struct hwclk_time t;
-
- if (con_fd < 0) { /* first time here */
- con_fd_filename = "/dev/tty1";
- con_fd = open(con_fd_filename, O_RDONLY);
- }
- if (con_fd < 0) {
- /* perhaps they are using devfs? */
- con_fd_filename = "/dev/vc/1";
- con_fd = open(con_fd_filename, O_RDONLY);
- }
- if (con_fd < 0) {
- /* probably KDGHWCLK exists on m68k only */
- warn(_("Can't open /dev/tty1 or /dev/vc/1"));
- } else {
- if (ioctl(con_fd, KDGHWCLK, &t) == -1) {
- if (errno != EINVAL)
- warn(_("KDGHWCLK ioctl failed"));
- } else
- ret = &kd;
- }
- return ret;
-}
-#endif /* __m68k__ && KDGHWCLK */
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 474e04f..d0d422d 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1256,9 +1256,6 @@ static void determine_clock_access_method(const bool user_requests_ISA)
ur = probe_for_rtc_clock();
#endif
- if (!ur)
- ur = probe_for_kd_clock();
-
if (!ur && !user_requests_ISA)
ur = probe_for_cmos_clock();
diff --git a/sys-utils/hwclock.h b/sys-utils/hwclock.h
index 69b0ce2..974d96a 100644
--- a/sys-utils/hwclock.h
+++ b/sys-utils/hwclock.h
@@ -19,7 +19,6 @@ struct clock_ops {
extern struct clock_ops *probe_for_cmos_clock(void);
extern struct clock_ops *probe_for_rtc_clock(void);
-extern struct clock_ops *probe_for_kd_clock(void);
typedef int bool;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 15/17] setarch: reindent code
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (13 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 14/17] hwclock: remove referal to deprecated keyboard interface Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-07 12:43 ` [PATCH 16/17] setarch: use personality() system call when it is available Sami Kerola
` (3 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/setarch.c | 507 +++++++++++++++++++++++++---------------------------
1 file changed, 248 insertions(+), 259 deletions(-)
diff --git a/sys-utils/setarch.c b/sys-utils/setarch.c
index 4cce877..468e492 100644
--- a/sys-utils/setarch.c
+++ b/sys-utils/setarch.c
@@ -41,7 +41,7 @@
/* Options without equivalent short options */
enum {
- OPT_4GB = CHAR_MAX + 1,
+ OPT_4GB = CHAR_MAX + 1,
OPT_UNAME26
};
@@ -52,7 +52,6 @@ enum {
printf(_("Switching on %s.\n"), #_flag); \
} while(0)
-
#ifndef UNAME26
# define UNAME26 0x0020000
#endif
@@ -87,301 +86,291 @@ enum {
# define ADDR_LIMIT_3GB 0x8000000
#endif
-static void __attribute__((__noreturn__))
-show_help(void)
+static void __attribute__((__noreturn__)) show_help(void)
{
- fputs(USAGE_HEADER, stdout);
- printf(_(" %s%s [options] [program [program arguments]]\n"),
- program_invocation_short_name,
- !strcmp(program_invocation_short_name, "setarch") ? " <arch>" : "");
-
- fputs(USAGE_OPTIONS, stdout);
- fputs(_(" -v, --verbose says what options are being switched on\n"), stdout);
- fputs(_(" -R, --addr-no-randomize disables randomization of the virtual address space\n"), stdout);
- fputs(_(" -F, --fdpic-funcptrs makes function pointers point to descriptors\n"), stdout);
- fputs(_(" -Z, --mmap-page-zero turns on MMAP_PAGE_ZERO\n"), stdout);
- fputs(_(" -L, --addr-compat-layout changes the way virtual memory is allocated\n"), stdout);
- fputs(_(" -X, --read-implies-exec turns on READ_IMPLIES_EXEC\n"), stdout);
- fputs(_(" -B, --32bit turns on ADDR_LIMIT_32BIT\n"), stdout);
- fputs(_(" -I, --short-inode turns on SHORT_INODE\n"), stdout);
- fputs(_(" -S, --whole-seconds turns on WHOLE_SECONDS\n"), stdout);
- fputs(_(" -T, --sticky-timeouts turns on STICKY_TIMEOUTS\n"), stdout);
- fputs(_(" -3, --3gb limits the used address space to a maximum of 3 GB\n"), stdout);
- fputs(_(" --4gb ignored (for backward compatibility only)\n"), stdout);
- fputs(_(" --uname-2.6 turns on UNAME26\n"), stdout);
- fputs(_(" --list list settable architectures, and exit\n"), stdout);
-
- fputs(USAGE_SEPARATOR, stdout);
- fputs(USAGE_HELP, stdout);
- fputs(USAGE_VERSION, stdout);
- printf(USAGE_MAN_TAIL("setarch(8)"));
-
- exit(EXIT_SUCCESS);
+ fputs(USAGE_HEADER, stdout);
+ printf(_(" %s%s [options] [program [program arguments]]\n"),
+ program_invocation_short_name,
+ !strcmp(program_invocation_short_name, "setarch") ? " <arch>" : "");
+ fputs(USAGE_OPTIONS, stdout);
+ fputs(_(" -v, --verbose says what options are being switched on\n"), stdout);
+ fputs(_(" -R, --addr-no-randomize disables randomization of the virtual address space\n"), stdout);
+ fputs(_(" -F, --fdpic-funcptrs makes function pointers point to descriptors\n"), stdout);
+ fputs(_(" -Z, --mmap-page-zero turns on MMAP_PAGE_ZERO\n"), stdout);
+ fputs(_(" -L, --addr-compat-layout changes the way virtual memory is allocated\n"), stdout);
+ fputs(_(" -X, --read-implies-exec turns on READ_IMPLIES_EXEC\n"), stdout);
+ fputs(_(" -B, --32bit turns on ADDR_LIMIT_32BIT\n"), stdout);
+ fputs(_(" -I, --short-inode turns on SHORT_INODE\n"), stdout);
+ fputs(_(" -S, --whole-seconds turns on WHOLE_SECONDS\n"), stdout);
+ fputs(_(" -T, --sticky-timeouts turns on STICKY_TIMEOUTS\n"), stdout);
+ fputs(_(" -3, --3gb limits the used address space to a maximum of 3 GB\n"), stdout);
+ fputs(_(" --4gb ignored (for backward compatibility only)\n"), stdout);
+ fputs(_(" --uname-2.6 turns on UNAME26\n"), stdout);
+ fputs(_(" --list list settable architectures, and exit\n"), stdout);
+ fputs(USAGE_SEPARATOR, stdout);
+ fputs(USAGE_HELP, stdout);
+ fputs(USAGE_VERSION, stdout);
+ printf(USAGE_MAN_TAIL("setarch(8)"));
+ exit(EXIT_SUCCESS);
}
-static void __attribute__((__noreturn__))
-show_usage(const char *s)
+static void __attribute__((__noreturn__)) show_usage(const char *s)
{
- if (s)
- errx(EXIT_FAILURE, _("%s\nTry `%s --help' for more information."), s, program_invocation_short_name);
- else
- errx(EXIT_FAILURE, _("Try `%s --help' for more information."), program_invocation_short_name);
+ if (s)
+ errx(EXIT_FAILURE,
+ _("%s\nTry `%s --help' for more information."), s,
+ program_invocation_short_name);
+ else
+ errx(EXIT_FAILURE, _("Try `%s --help' for more information."),
+ program_invocation_short_name);
}
static void __attribute__((__noreturn__))
-show_version(void)
+ show_version(void)
{
- printf(UTIL_LINUX_VERSION);
- exit(EXIT_SUCCESS);
+ printf(UTIL_LINUX_VERSION);
+ exit(EXIT_SUCCESS);
}
-static int
-set_arch(const char *pers, unsigned long options, int list)
+static int set_arch(const char *pers, unsigned long options, int list)
{
- struct utsname un;
- int i;
- unsigned long pers_value;
-
- struct {
- int perval;
- const char *target_arch, *result_arch;
- } transitions[] = {
- {UNAME26, "uname26", NULL},
- {PER_LINUX32, "linux32", NULL},
- {PER_LINUX, "linux64", NULL},
+ struct utsname un;
+ int i;
+ unsigned long pers_value;
+
+ struct {
+ int perval;
+ const char *target_arch, *result_arch;
+ } transitions[] = {
+ {UNAME26, "uname26", NULL},
+ {PER_LINUX32, "linux32", NULL},
+ {PER_LINUX, "linux64", NULL},
#if defined(__powerpc__) || defined(__powerpc64__)
-#ifdef __BIG_ENDIAN__
- {PER_LINUX32, "ppc32", "ppc"},
- {PER_LINUX32, "ppc", "ppc"},
- {PER_LINUX, "ppc64", "ppc64"},
- {PER_LINUX, "ppc64pseries", "ppc64"},
- {PER_LINUX, "ppc64iseries", "ppc64"},
-#else
- {PER_LINUX32, "ppc32le", "ppcle"},
- {PER_LINUX32, "ppcle", "ppcle"},
- {PER_LINUX, "ppc64le", "ppc64le"},
-#endif
+# ifdef __BIG_ENDIAN__
+ {PER_LINUX32, "ppc32", "ppc"},
+ {PER_LINUX32, "ppc", "ppc"},
+ {PER_LINUX, "ppc64", "ppc64"},
+ {PER_LINUX, "ppc64pseries", "ppc64"},
+ {PER_LINUX, "ppc64iseries", "ppc64"},
+# else
+ PER_LINUX32, "ppc32le", "ppcle"},
+ PER_LINUX32, "ppcle", "ppcle"},
+ PER_LINUX, "ppc64le", "ppc64le"},
+# endif
#endif
#if defined(__x86_64__) || defined(__i386__) || defined(__ia64__)
- {PER_LINUX32, "i386", "i386"},
- {PER_LINUX32, "i486", "i386"},
- {PER_LINUX32, "i586", "i386"},
- {PER_LINUX32, "i686", "i386"},
- {PER_LINUX32, "athlon", "i386"},
+ {PER_LINUX32, "i386", "i386"},
+ {PER_LINUX32, "i486", "i386"},
+ {PER_LINUX32, "i586", "i386"},
+ {PER_LINUX32, "i686", "i386"},
+ {PER_LINUX32, "athlon", "i386"},
#endif
#if defined(__x86_64__) || defined(__i386__)
- {PER_LINUX, "x86_64", "x86_64"},
+ {PER_LINUX, "x86_64", "x86_64"},
#endif
#if defined(__ia64__) || defined(__i386__)
- {PER_LINUX, "ia64", "ia64"},
+ {PER_LINUX, "ia64", "ia64"},
#endif
#if defined(__hppa__)
- {PER_LINUX32, "parisc32", "parisc"},
- {PER_LINUX32, "parisc", "parisc"},
- {PER_LINUX, "parisc64", "parisc64"},
+ {PER_LINUX32, "parisc32", "parisc"},
+ {PER_LINUX32, "parisc", "parisc"},
+ {PER_LINUX, "parisc64", "parisc64"},
#endif
#if defined(__s390x__) || defined(__s390__)
- {PER_LINUX32, "s390", "s390"},
- {PER_LINUX, "s390x", "s390x"},
+ {PER_LINUX32, "s390", "s390"},
+ {PER_LINUX, "s390x", "s390x"},
#endif
#if defined(__sparc64__) || defined(__sparc__)
- {PER_LINUX32, "sparc", "sparc"},
- {PER_LINUX32, "sparc32bash", "sparc"},
- {PER_LINUX32, "sparc32", "sparc"},
- {PER_LINUX, "sparc64", "sparc64"},
+ {PER_LINUX32, "sparc", "sparc"},
+ {PER_LINUX32, "sparc32bash", "sparc"},
+ {PER_LINUX32, "sparc32", "sparc"},
+ {PER_LINUX, "sparc64", "sparc64"},
#endif
#if defined(__mips64__) || defined(__mips__)
- {PER_LINUX32, "mips32", "mips"},
- {PER_LINUX32, "mips", "mips"},
- {PER_LINUX, "mips64", "mips64"},
+ {PER_LINUX32, "mips32", "mips"},
+ {PER_LINUX32, "mips", "mips"},
+ {PER_LINUX, "mips64", "mips64"},
#endif
#if defined(__alpha__)
- {PER_LINUX, "alpha", "alpha"},
- {PER_LINUX, "alphaev5", "alpha"},
- {PER_LINUX, "alphaev56", "alpha"},
- {PER_LINUX, "alphaev6", "alpha"},
- {PER_LINUX, "alphaev67", "alpha"},
+ {PER_LINUX, "alpha", "alpha"},
+ {PER_LINUX, "alphaev5", "alpha"},
+ {PER_LINUX, "alphaev56", "alpha"},
+ {PER_LINUX, "alphaev6", "alpha"},
+ {PER_LINUX, "alphaev67", "alpha"},
#endif
- {-1, NULL, NULL}, /* place holder, eventually filled up at runtime */
- {-1, NULL, NULL}
- };
-
- /* Add the trivial transition {PER_LINUX, machine, machine} if no such
- target_arch is hardcoded yet. */
- uname(&un);
- for (i = 0; transitions[i].perval >= 0; i++)
- if(!strcmp(un.machine, transitions[i].target_arch))
- break;
- if (transitions[i].perval < 0) {
- unsigned long wrdsz = CHAR_BIT * sizeof(void*);
- if (wrdsz == 32 || wrdsz == 64) {
- transitions[i].perval = wrdsz == 32 ? PER_LINUX32 : PER_LINUX;
- transitions[i].target_arch = un.machine;
- transitions[i].result_arch = un.machine;
+ /* place holder, will be filled up at runtime */
+ {-1, NULL, NULL},
+ {-1, NULL, NULL}
+ };
+
+ /* Add the trivial transition {PER_LINUX, machine, machine} if no
+ * such target_arch is hardcoded yet. */
+ uname(&un);
+ for (i = 0; transitions[i].perval >= 0; i++)
+ if (!strcmp(un.machine, transitions[i].target_arch))
+ break;
+ if (transitions[i].perval < 0) {
+ unsigned long wrdsz = CHAR_BIT * sizeof(void *);
+ if (wrdsz == 32 || wrdsz == 64) {
+ /* fill up the place holder */
+ transitions[i].perval = wrdsz == 32 ? PER_LINUX32 : PER_LINUX;
+ transitions[i].target_arch = un.machine;
+ transitions[i].result_arch = un.machine;
+ }
}
- }
-
- if (list) {
- for(i = 0; transitions[i].target_arch != NULL; i++)
- printf("%s\n", transitions[i].target_arch);
- return 0;
- }
-
- for(i = 0; transitions[i].perval >= 0; i++)
- if(!strcmp(pers, transitions[i].target_arch))
- break;
-
- if(transitions[i].perval < 0)
- errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
-
- pers_value = transitions[i].perval | options;
- if (set_pers(pers_value) == -EINVAL)
- return 1;
-
- uname(&un);
- if(transitions[i].result_arch &&
- strcmp(un.machine, transitions[i].result_arch))
- {
- if(strcmp(transitions[i].result_arch, "i386")
- || (strcmp(un.machine, "i486")
- && strcmp(un.machine, "i586")
- && strcmp(un.machine, "i686")
- && strcmp(un.machine, "athlon")))
- errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
- }
-
- return 0;
+ if (list) {
+ for (i = 0; transitions[i].target_arch != NULL; i++)
+ printf("%s\n", transitions[i].target_arch);
+ return 0;
+ }
+ for (i = 0; transitions[i].perval >= 0; i++)
+ if (!strcmp(pers, transitions[i].target_arch))
+ break;
+ if (transitions[i].perval < 0)
+ errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
+ pers_value = transitions[i].perval | options;
+ if (set_pers(pers_value) == -EINVAL)
+ return 1;
+ uname(&un);
+ if (transitions[i].result_arch && strcmp(un.machine, transitions[i].result_arch)) {
+ if (strcmp(transitions[i].result_arch, "i386")
+ || (strcmp(un.machine, "i486")
+ && strcmp(un.machine, "i586")
+ && strcmp(un.machine, "i686")
+ && strcmp(un.machine, "athlon")))
+ errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
+ }
+ return 0;
}
int main(int argc, char *argv[])
{
- const char *p;
- unsigned long options = 0;
- int verbose = 0;
- int c;
-
- /* Options --3gb and --4gb are for compatibitity with an old Debian setarch
- implementation. */
- static const struct option longopts[] =
- {
- { "help", 0, 0, 'h' },
- { "version", 0, 0, 'V' },
- { "verbose", 0, 0, 'v' },
- { "addr-no-randomize", 0, 0, 'R' },
- { "fdpic-funcptrs", 0, 0, 'F' },
- { "mmap-page-zero", 0, 0, 'Z' },
- { "addr-compat-layout", 0, 0, 'L' },
- { "read-implies-exec", 0, 0, 'X' },
- { "32bit", 0, 0, 'B' },
- { "short-inode", 0, 0, 'I' },
- { "whole-seconds", 0, 0, 'S' },
- { "sticky-timeouts", 0, 0, 'T' },
- { "3gb", 0, 0, '3' },
- { "4gb", 0, 0, OPT_4GB },
- { "uname-2.6", 0, 0, OPT_UNAME26 },
- { NULL, 0, 0, 0 }
- };
-
- setlocale(LC_ALL, "");
- bindtextdomain(PACKAGE, LOCALEDIR);
- textdomain(PACKAGE);
- atexit(close_stdout);
-
- if (argc < 1)
- show_usage(_("Not enough arguments"));
-
- p = program_invocation_short_name;
- if (!strcmp(p, "setarch")) {
- argc--;
- if (argc < 1)
- show_usage(_("Not enough arguments"));
- p = argv[1];
- argv[1] = argv[0]; /* for getopt_long() to get the program name */
- argv++;
- if (!strcmp(p, "-h") || !strcmp(p, "--help"))
- show_help();
- else if (!strcmp(p, "-V") || !strcmp(p, "--version"))
- show_version();
- else if (!strcmp(p, "--list")) {
- set_arch(argv[0], 0L, 1);
- return EXIT_SUCCESS;
- }
- }
- #if defined(__sparc64__) || defined(__sparc__)
- if (!strcmp(p, "sparc32bash")) {
- if (set_arch(p, 0L, 0))
- err(EXIT_FAILURE, _("Failed to set personality to %s"), p);
- execl("/bin/bash", NULL);
- err(EXIT_FAILURE, _("failed to execute %s"), "/bin/bash");
- }
- #endif
+ const char *p;
+ unsigned long options = 0;
+ int verbose = 0;
+ int c;
+
+ /* Options --3gb and --4gb are for compatibitity with an old
+ * Debian setarch implementation. */
+ static const struct option longopts[] = {
+ {"help", no_argument, NULL, 'h'},
+ {"version", no_argument, NULL, 'V'},
+ {"verbose", no_argument, NULL, 'v'},
+ {"addr-no-randomize", no_argument, NULL, 'R'},
+ {"fdpic-funcptrs", no_argument, NULL, 'F'},
+ {"mmap-page-zero", no_argument, NULL, 'Z'},
+ {"addr-compat-layout", no_argument, NULL, 'L'},
+ {"read-implies-exec", no_argument, NULL, 'X'},
+ {"32bit", no_argument, NULL, 'B'},
+ {"short-inode", no_argument, NULL, 'I'},
+ {"whole-seconds", no_argument, NULL, 'S'},
+ {"sticky-timeouts", no_argument, NULL, 'T'},
+ {"3gb", no_argument, NULL, '3'},
+ {"4gb", no_argument, NULL, OPT_4GB},
+ {"uname-2.6", no_argument, NULL, OPT_UNAME26},
+ {NULL, 0, NULL, 0}
+ };
+
+ setlocale(LC_ALL, "");
+ bindtextdomain(PACKAGE, LOCALEDIR);
+ textdomain(PACKAGE);
+ atexit(close_stdout);
+
+ if (argc < 1)
+ show_usage(_("Not enough arguments"));
+
+ p = program_invocation_short_name;
+ if (!strcmp(p, "setarch")) {
+ argc--;
+ if (argc < 1)
+ show_usage(_("Not enough arguments"));
+ p = argv[1];
+ argv[1] = argv[0]; /* for getopt_long() to get the program name */
+ argv++;
+ if (!strcmp(p, "-h") || !strcmp(p, "--help"))
+ show_help();
+ else if (!strcmp(p, "-V") || !strcmp(p, "--version"))
+ show_version();
+ else if (!strcmp(p, "--list")) {
+ set_arch(argv[0], 0L, 1);
+ return EXIT_SUCCESS;
+ }
+ }
+#if defined(__sparc64__) || defined(__sparc__)
+ if (!strcmp(p, "sparc32bash")) {
+ if (set_arch(p, 0L, 0))
+ err(EXIT_FAILURE, _("Failed to set personality to %s"), p);
+ execl("/bin/bash", NULL);
+ err(EXIT_FAILURE, _("failed to execute %s"), "/bin/bash");
+ }
+#endif
- while ((c = getopt_long(argc, argv, "+hVv3BFILRSTXZ", longopts, NULL)) != -1) {
- switch (c) {
- case 'h':
- show_help();
- break;
- case 'V':
- show_version();
- break;
- case 'v':
- verbose = 1;
- break;
- case 'R':
- turn_on(ADDR_NO_RANDOMIZE, options);
- break;
- case 'F':
- turn_on(FDPIC_FUNCPTRS, options);
- break;
- case 'Z':
- turn_on(MMAP_PAGE_ZERO, options);
- break;
- case 'L':
- turn_on(ADDR_COMPAT_LAYOUT, options);
- break;
- case 'X':
- turn_on(READ_IMPLIES_EXEC, options);
- break;
- case 'B':
- turn_on(ADDR_LIMIT_32BIT, options);
- break;
- case 'I':
- turn_on(SHORT_INODE, options);
- break;
- case 'S':
- turn_on(WHOLE_SECONDS, options);
- break;
- case 'T':
- turn_on(STICKY_TIMEOUTS, options);
- break;
- case '3':
- turn_on(ADDR_LIMIT_3GB, options);
- break;
- case OPT_4GB: /* just ignore this one */
- break;
- case OPT_UNAME26:
- turn_on(UNAME26, options);
- break;
- default:
- show_usage(NULL);
- }
- }
+ while ((c = getopt_long(argc, argv, "+hVv3BFILRSTXZ", longopts, NULL)) != -1) {
+ switch (c) {
+ case 'h':
+ show_help();
+ break;
+ case 'V':
+ show_version();
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ case 'R':
+ turn_on(ADDR_NO_RANDOMIZE, options);
+ break;
+ case 'F':
+ turn_on(FDPIC_FUNCPTRS, options);
+ break;
+ case 'Z':
+ turn_on(MMAP_PAGE_ZERO, options);
+ break;
+ case 'L':
+ turn_on(ADDR_COMPAT_LAYOUT, options);
+ break;
+ case 'X':
+ turn_on(READ_IMPLIES_EXEC, options);
+ break;
+ case 'B':
+ turn_on(ADDR_LIMIT_32BIT, options);
+ break;
+ case 'I':
+ turn_on(SHORT_INODE, options);
+ break;
+ case 'S':
+ turn_on(WHOLE_SECONDS, options);
+ break;
+ case 'T':
+ turn_on(STICKY_TIMEOUTS, options);
+ break;
+ case '3':
+ turn_on(ADDR_LIMIT_3GB, options);
+ break;
+ case OPT_4GB: /* just ignore this one */
+ break;
+ case OPT_UNAME26:
+ turn_on(UNAME26, options);
+ break;
+ default:
+ show_usage(NULL);
+ }
+ }
- argc -= optind;
- argv += optind;
+ argc -= optind;
+ argv += optind;
- if (set_arch(p, options, 0))
- err(EXIT_FAILURE, _("Failed to set personality to %s"), p);
+ if (set_arch(p, options, 0))
+ err(EXIT_FAILURE, _("Failed to set personality to %s"), p);
- /* flush all output streams before exec */
- fflush(NULL);
+ /* flush all output streams before exec */
+ fflush(NULL);
- if (!argc) {
- execl("/bin/sh", "-sh", NULL);
- err(EXIT_FAILURE, _("failed to execute %s"), "/bin/sh");
- }
+ if (!argc) {
+ execl("/bin/sh", "-sh", NULL);
+ err(EXIT_FAILURE, _("failed to execute %s"), "/bin/sh");
+ }
- execvp(argv[0], argv);
- err(EXIT_FAILURE, "%s", argv[0]);
- return EXIT_FAILURE;
+ execvp(argv[0], argv);
+ err(EXIT_FAILURE, "%s", argv[0]);
+ return EXIT_FAILURE;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 16/17] setarch: use personality() system call when it is available
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (14 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 15/17] setarch: reindent code Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-07 12:43 ` [PATCH 17/17] setarch: remove unreachable code Sami Kerola
` (2 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/setarch.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sys-utils/setarch.c b/sys-utils/setarch.c
index 468e492..ed2a6a6 100644
--- a/sys-utils/setarch.c
+++ b/sys-utils/setarch.c
@@ -24,8 +24,7 @@
* sparc32 util by Jakub Jelinek (1998, 1999)
*/
-#include <syscall.h>
-#include <linux/personality.h>
+#include <sys/personality.h>
#include <unistd.h>
#include <stdio.h>
#include <string.h>
@@ -37,7 +36,10 @@
#include "c.h"
#include "closestream.h"
-#define set_pers(pers) ((long)syscall(SYS_personality, pers))
+#ifndef HAVE_PERSONALITY
+# include <syscall.h>
+# define personality(pers) ((long)syscall(SYS_personality, pers))
+#endif
/* Options without equivalent short options */
enum {
@@ -229,7 +231,7 @@ static int set_arch(const char *pers, unsigned long options, int list)
if (transitions[i].perval < 0)
errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
pers_value = transitions[i].perval | options;
- if (set_pers(pers_value) == -EINVAL)
+ if (personality(pers_value) == -EINVAL)
return 1;
uname(&un);
if (transitions[i].result_arch && strcmp(un.machine, transitions[i].result_arch)) {
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 17/17] setarch: remove unreachable code
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (15 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 16/17] setarch: use personality() system call when it is available Sami Kerola
@ 2014-09-07 12:43 ` Sami Kerola
2014-09-12 9:50 ` Karel Zak
2014-09-16 20:56 ` [PATCH 00/17] pull: miscellaneous changes Sami Kerola
2014-09-22 13:01 ` Karel Zak
18 siblings, 1 reply; 29+ messages in thread
From: Sami Kerola @ 2014-09-07 12:43 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Architectures are protected using '#if defined' clauses, making the code
in this removal to be impossible to reach.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/setarch.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/sys-utils/setarch.c b/sys-utils/setarch.c
index ed2a6a6..43594ad 100644
--- a/sys-utils/setarch.c
+++ b/sys-utils/setarch.c
@@ -233,15 +233,6 @@ static int set_arch(const char *pers, unsigned long options, int list)
pers_value = transitions[i].perval | options;
if (personality(pers_value) == -EINVAL)
return 1;
- uname(&un);
- if (transitions[i].result_arch && strcmp(un.machine, transitions[i].result_arch)) {
- if (strcmp(transitions[i].result_arch, "i386")
- || (strcmp(un.machine, "i486")
- && strcmp(un.machine, "i586")
- && strcmp(un.machine, "i686")
- && strcmp(un.machine, "athlon")))
- errx(EXIT_FAILURE, _("%s: Unrecognized architecture"), pers);
- }
return 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 17/17] setarch: remove unreachable code
2014-09-07 12:43 ` [PATCH 17/17] setarch: remove unreachable code Sami Kerola
@ 2014-09-12 9:50 ` Karel Zak
2014-09-16 20:50 ` Sami Kerola
0 siblings, 1 reply; 29+ messages in thread
From: Karel Zak @ 2014-09-12 9:50 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Sep 07, 2014 at 01:43:05PM +0100, Sami Kerola wrote:
> Architectures are protected using '#if defined' clauses, making the code
> in this removal to be impossible to reach.
Hmmm.. I guess the code tries to verify that the result architecture
after personality() matches with uname.machine (except to
i{486,586,...}), to be sure that our request has been correctly
interpreted by kernel.
IMHO it's correct.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 17/17] setarch: remove unreachable code
2014-09-12 9:50 ` Karel Zak
@ 2014-09-16 20:50 ` Sami Kerola
0 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-16 20:50 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sun, Sep 07, 2014 at 01:43:05PM +0100, Sami Kerola wrote:
>> Architectures are protected using '#if defined' clauses, making the code
>> in this removal to be impossible to reach.
>
> Hmmm.. I guess the code tries to verify that the result architecture
> after personality() matches with uname.machine (except to
> i{486,586,...}), to be sure that our request has been correctly
> interpreted by kernel.
>
> IMHO it's correct.
I removed the change from incoming pull request, and try to improve the
'Unrecognized architecture' message this week. Since the condition looks
impossible reach maybe the message should encourage to report to
util-linux maillist with which transitions[i].result_arch and un.machine
values one was able to see it.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 00/17] pull: miscellaneous changes
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (16 preceding siblings ...)
2014-09-07 12:43 ` [PATCH 17/17] setarch: remove unreachable code Sami Kerola
@ 2014-09-16 20:56 ` Sami Kerola
2014-09-22 13:01 ` Karel Zak
18 siblings, 0 replies; 29+ messages in thread
From: Sami Kerola @ 2014-09-16 20:56 UTC (permalink / raw)
To: util-linux
On Sun, 7 Sep 2014, Sami Kerola wrote:
> This patch set is full of random small changes all around the util-linux
> code base. I am pretty sure all changes are good, but it would be really
> good if someone could double check the last one of the changes, setarch:
> remove unreachable code, is correct.
>
> While doing eject changes I noticed eject(1) executing umount(8). That
> works, but shouldn't the code be changed to use libmount? Or is that a
> pointless complication?
Hello,
Here is rebased and updated version of the pull request. Most of changes
are with newgrp(1). The command started to use getsgnam() to read gshadow,
and the xgetpass() stuff is removed from util-linux/lib.
The following changes since commit a3b92242ad76a7468cf508e1d878d0815c7e031f:
libmount: hide details about failed search in fstab/mtab (2014-09-16 15:30:03 +0200)
are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git misc
for you to fetch changes up to fabf9766ddfb44f75165298928d3fcba44d01d5e:
textual: share crypt() error message in sulogin and newgrp (2014-09-16
21:09:19 +0100)
----------------------------------------------------------------
Sami Kerola (17):
libuuid: add extern qualifiers to uuid/uuid.h system header
include: simplify fputc_careful() in carefulputc.h
lib: remove xgetpass()
last: make is_phantom() when kernel config does not include audit support
last: improve code readability by renaming variable names
zramctl: fix two format string warnings
mountpoint: add struct mountpoint_control
mkfs.minix: fix couple compiler warnings
mountpoint: simplify if statement
eject: add struct eject_control to remove global variables
eject: make open_device() and select_speed() to use struct eject_control
hwclock: remove referal to deprecated keyboard interface
setarch: reindent code
setarch: use personality() system call when it is available
newgrp: use libc function to read gshadow if it is available
newgrp: avoid use of obsolete getpass() function
textual: share crypt() error message in sulogin and newgrp
configure.ac | 1 +
disk-utils/mkfs.minix.c | 16 +-
include/Makemodule.am | 1 -
include/carefulputc.h | 16 +-
include/xgetpass.h | 6 -
lib/Makemodule.am | 1 -
lib/xgetpass.c | 46 -----
libuuid/src/uuid.h | 30 +--
login-utils/last.c | 53 +++--
login-utils/newgrp.c | 61 +++++-
sys-utils/Makemodule.am | 3 +-
sys-utils/eject.c | 422 ++++++++++++++++++++-------------------
sys-utils/hwclock-kd.c | 180 -----------------
sys-utils/hwclock.c | 3 -
sys-utils/hwclock.h | 1 -
sys-utils/mountpoint.c | 93 ++++-----
sys-utils/setarch.c | 515 ++++++++++++++++++++++++------------------------
sys-utils/zramctl.c | 4 +-
18 files changed, 633 insertions(+), 819 deletions(-)
delete mode 100644 include/xgetpass.h
delete mode 100644 lib/xgetpass.c
delete mode 100644 sys-utils/hwclock-kd.c
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/17] pull: miscellaneous changes
2014-09-07 12:42 [PATCH 00/17] pull: miscellaneous changes Sami Kerola
` (17 preceding siblings ...)
2014-09-16 20:56 ` [PATCH 00/17] pull: miscellaneous changes Sami Kerola
@ 2014-09-22 13:01 ` Karel Zak
18 siblings, 0 replies; 29+ messages in thread
From: Karel Zak @ 2014-09-22 13:01 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sun, Sep 07, 2014 at 01:42:48PM +0100, Sami Kerola wrote:
> git://github.com/kerolasa/lelux-utiliteetit.git misc
Merged, thanks.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 29+ messages in thread