* [PATCH 00/16] pull: changes to various utils
@ 2015-02-22 14:41 Sami Kerola
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
` (17 more replies)
0 siblings, 18 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Hello,
Here is collection of patches that hit various utilities all around
source tree, and do not have any particular theme. These changes are
also available from my github remote repository.
git://github.com/kerolasa/lelux-utiliteetit.git various
Sami Kerola (16):
whereis: tell when mandatory option is missing
flock: add --verbose option
flock: improve timeout handling
prlimit: tell in --verbose output which pid got the new limit
tunelp: remove get_val() in favour of strtol_or_err()
tunelp: remove unnecessary preprocessor directives
lib/strutils: move parse_switch() from setterm(1) to library
tunelp: use parse_switch()
eject: use parse_switch()
rpmatch: use symbolic value when evaluation return codes
tailf: check printing criteria more carefully
tailf: count last lines correctly at initial print out
tailf: do not allow minus signed last lines argument
tailf: ensure file argument really is a file
logger: move /dev/log to pathnames.h
logger: fix -i argument parsing regression
disk-utils/fdisk.c | 2 +-
disk-utils/fsck.minix.c | 6 +--
disk-utils/sfdisk.c | 2 +-
include/Makemodule.am | 1 -
include/pathnames.h | 3 ++
include/rpmatch.h | 4 ++
include/strutils.h | 2 +
include/timer.h | 31 -----------
lib/strutils.c | 8 +++
login-utils/vipw.c | 2 +-
misc-utils/logger.1 | 5 +-
misc-utils/logger.c | 16 ++++--
misc-utils/whereis.c | 8 ++-
sys-utils/Makemodule.am | 5 +-
sys-utils/eject.c | 14 +----
sys-utils/flock.1 | 4 ++
sys-utils/flock.c | 82 +++++++++++++++++++++++++----
sys-utils/prlimit.c | 3 +-
sys-utils/tunelp.c | 51 +++++-------------
term-utils/mesg.c | 6 +--
term-utils/setterm.c | 14 -----
text-utils/tailf.c | 135 ++++++++++++++++++++++++++++--------------------
22 files changed, 224 insertions(+), 180 deletions(-)
delete mode 100644 include/timer.h
--
2.3.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 01/16] whereis: tell when mandatory option is missing
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 17:37 ` Benno Schulenberg
2015-02-24 9:40 ` Karel Zak
2015-02-22 14:41 ` [PATCH 02/16] flock: add --verbose option Sami Kerola
` (16 subsequent siblings)
17 siblings, 2 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/whereis.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/misc-utils/whereis.c b/misc-utils/whereis.c
index 9fc5ee7..c47e122 100644
--- a/misc-utils/whereis.c
+++ b/misc-utils/whereis.c
@@ -495,7 +495,7 @@ int main(int argc, char **argv)
{
struct wh_dirlist *ls = NULL;
int want = ALL_DIRS;
- int i, want_resetable = 0;
+ int i, want_resetable = 0, opt_f_missing = 0;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
@@ -540,6 +540,7 @@ int main(int argc, char **argv)
switch (*arg) {
case 'f':
+ opt_f_missing = 0;
break;
case 'u':
uflag = 1;
@@ -551,6 +552,7 @@ int main(int argc, char **argv)
free_dirlist(&ls, BIN_DIR);
construct_dirlist_from_argv(
&ls, &i, argc, argv, BIN_DIR);
+ opt_f_missing = 1;
break;
case 'M':
if (*(arg + 1))
@@ -559,6 +561,7 @@ int main(int argc, char **argv)
free_dirlist(&ls, MAN_DIR);
construct_dirlist_from_argv(
&ls, &i, argc, argv, MAN_DIR);
+ opt_f_missing = 1;
break;
case 'S':
if (*(arg + 1))
@@ -567,6 +570,7 @@ int main(int argc, char **argv)
free_dirlist(&ls, SRC_DIR);
construct_dirlist_from_argv(
&ls, &i, argc, argv, SRC_DIR);
+ opt_f_missing = 1;
break;
case 'b':
if (want_resetable) {
@@ -607,5 +611,7 @@ int main(int argc, char **argv)
}
free_dirlist(&ls, ALL_DIRS);
+ if (opt_f_missing)
+ errx(EXIT_FAILURE, _("option -f missing"));
return EXIT_SUCCESS;
}
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 02/16] flock: add --verbose option
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 17:44 ` Benno Schulenberg
2015-02-22 14:41 ` [PATCH 03/16] flock: improve timeout handling Sami Kerola
` (15 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Jenkins script jobs using flock are a great example of a situation in
which one may want an automation to be verbose, so that when unexpected
events happen there is more hints in logs.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/Makemodule.am | 4 ++--
sys-utils/flock.1 | 4 ++++
sys-utils/flock.c | 33 ++++++++++++++++++++++++++++++---
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index 15d0a95..d3f77b4 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -1,8 +1,8 @@
if BUILD_FLOCK
usrbin_exec_PROGRAMS += flock
dist_man_MANS += sys-utils/flock.1
-flock_SOURCES = sys-utils/flock.c
-flock_LDADD = $(LDADD) libcommon.la
+flock_SOURCES = sys-utils/flock.c lib/monotonic.c
+flock_LDADD = $(LDADD) libcommon.la $(CLOCKGETTIME_LIBS)
endif
if BUILD_IPCMK
diff --git a/sys-utils/flock.1 b/sys-utils/flock.1
index cdc294b..f526b33 100644
--- a/sys-utils/flock.1
+++ b/sys-utils/flock.1
@@ -107,6 +107,10 @@ option for the exit code used. The zero number of
.IR seconds
is interpreted as \fB\-\-nonblock\fR.
.TP
+.B \-\-verbose
+Tell why lock could not be got, and when it can be how long did it take
+to acquire the lock.
+.TP
.BR \-V , " \-\-version"
Display version information and exit.
.TP
diff --git a/sys-utils/flock.c b/sys-utils/flock.c
index 3828155..cb8a82b 100644
--- a/sys-utils/flock.c
+++ b/sys-utils/flock.c
@@ -44,6 +44,7 @@
#include "strutils.h"
#include "closestream.h"
#include "timer.h"
+#include "monotonic.h"
static void __attribute__((__noreturn__)) usage(int ex)
{
@@ -66,6 +67,7 @@ static void __attribute__((__noreturn__)) usage(int ex)
fputs(_( " -E, --conflict-exit-code <number> exit code after conflict or timeout\n"), stderr);
fputs(_( " -o, --close close file descriptor before running command\n"), stderr);
fputs(_( " -c, --command <command> run a single command string through the shell\n"), stderr);
+ fputs(_( " --verbose increase verbosity\n"), stderr);
fprintf(stderr, USAGE_SEPARATOR);
fprintf(stderr, USAGE_HELP);
fprintf(stderr, USAGE_VERSION);
@@ -120,6 +122,8 @@ int main(int argc, char *argv[])
int opt, ix;
int do_close = 0;
int status;
+ int verbose = 0;
+ struct timeval time_start, time_done;
/*
* The default exit code for lock conflict or timeout
* is specified in man flock.1
@@ -128,7 +132,9 @@ int main(int argc, char *argv[])
char **cmd_argv = NULL, *sh_c_argv[4];
const char *filename = NULL;
struct sigaction old_sa;
-
+ enum {
+ OPT_VERBOSE = CHAR_MAX + 1
+ };
static const struct option long_options[] = {
{"shared", no_argument, NULL, 's'},
{"exclusive", no_argument, NULL, 'x'},
@@ -139,8 +145,9 @@ int main(int argc, char *argv[])
{"wait", required_argument, NULL, 'w'},
{"conflict-exit-code", required_argument, NULL, 'E'},
{"close", no_argument, NULL, 'o'},
+ {"verbose", no_argument, NULL, OPT_VERBOSE},
{"help", no_argument, NULL, 'h'},
- {"version", no_argument, NULL, 'V'},
+ {"version", required_argument, NULL, 'V'},
{NULL, 0, NULL, 0}
};
@@ -184,6 +191,9 @@ int main(int argc, char *argv[])
conflict_exit_code = strtos32_or_err(optarg,
_("invalid exit code"));
break;
+ case OPT_VERBOSE:
+ verbose = 1;
+ break;
case 'V':
printf(UTIL_LINUX_VERSION);
exit(EX_OK);
@@ -239,16 +249,23 @@ int main(int argc, char *argv[])
setup_timer(&timeout, &old_timer, &old_sa, timeout_handler);
}
+ if (verbose)
+ gettime_monotonic(&time_start);
while (flock(fd, type | block)) {
switch (errno) {
case EWOULDBLOCK:
/* -n option set and failed to lock. */
+ if (verbose)
+ warnx(_("failed to get lock"));
exit(conflict_exit_code);
case EINTR:
/* Signal received */
- if (timeout_expired)
+ if (timeout_expired) {
/* -w option set and failed to lock. */
+ if (verbose)
+ warnx(_("timeout while waiting to get lock"));
exit(conflict_exit_code);
+ }
/* otherwise try again */
continue;
case EIO:
@@ -282,13 +299,23 @@ int main(int argc, char *argv[])
if (have_timeout)
cancel_timer(&old_timer, &old_sa);
+ if (verbose) {
+ struct timeval delta;
+ gettime_monotonic(&time_done);
+ timersub(&time_done, &time_start, &delta);
+ printf(_("%s: getting lock took %ld.%06ld seconds\n"),
+ program_invocation_short_name, delta.tv_sec,
+ delta.tv_usec);
+ }
status = EX_OK;
if (cmd_argv) {
pid_t w, f;
/* Clear any inherited settings */
signal(SIGCHLD, SIG_DFL);
+ if (verbose)
+ printf(_("%s: executing %s\n"), program_invocation_short_name, cmd_argv[2]);
f = fork();
if (f < 0) {
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 03/16] flock: improve timeout handling
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
2015-02-22 14:41 ` [PATCH 02/16] flock: add --verbose option Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 10:21 ` Karel Zak
2015-03-05 9:36 ` Karel Zak
2015-02-22 14:41 ` [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit Sami Kerola
` (14 subsequent siblings)
17 siblings, 2 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signal ALRM raised by the timer, and the timer only, will be considered
as a timeout criteria.
Secondly time interval is made to use monotonic clock. Documentation of
ITIMER_REAL is unclear whether that time is affected various sources of
clock skew, or does it even tick when system is suspended.
This code is moved from libcommon.la to flock.c because of two reasons.
This is the only utility using the function, and setup_timer() along with
cancel_timer() need to be linked with -lrt option.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/Makemodule.am | 1 -
include/timer.h | 31 -------------------------------
sys-utils/Makemodule.am | 2 +-
sys-utils/flock.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
4 files changed, 43 insertions(+), 40 deletions(-)
delete mode 100644 include/timer.h
diff --git a/include/Makemodule.am b/include/Makemodule.am
index c4a52e4..8d7c881 100644
--- a/include/Makemodule.am
+++ b/include/Makemodule.am
@@ -44,7 +44,6 @@ dist_noinst_HEADERS += \
include/swapprober.h \
include/swapheader.h \
include/sysfs.h \
- include/timer.h \
include/timeutils.h \
include/ttyutils.h \
include/widechar.h \
diff --git a/include/timer.h b/include/timer.h
deleted file mode 100644
index 79ef649..0000000
--- a/include/timer.h
+++ /dev/null
@@ -1,31 +0,0 @@
-#ifndef UTIL_LINUX_TIMER_H
-#define UTIL_LINUX_TIMER_H
-
-#include <signal.h>
-#include <sys/time.h>
-
-static inline int setup_timer(
- struct itimerval *timer,
- struct itimerval *old_timer,
- struct sigaction *old_sa,
- void (*timeout_handler)(int))
-{
- struct sigaction sa;
-
- memset(&sa, 0, sizeof sa);
- sa.sa_handler = timeout_handler;
- sa.sa_flags = SA_RESETHAND;
- sigaction(SIGALRM, &sa, old_sa);
-
- return setitimer(ITIMER_REAL, timer, old_timer);
-}
-
-static inline void cancel_timer(
- struct itimerval *old_timer,
- struct sigaction *old_sa)
-{
- setitimer(ITIMER_REAL, old_timer, NULL);
- sigaction(SIGALRM, old_sa, NULL);
-}
-
-#endif
diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index d3f77b4..14d0773 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -2,7 +2,7 @@ if BUILD_FLOCK
usrbin_exec_PROGRAMS += flock
dist_man_MANS += sys-utils/flock.1
flock_SOURCES = sys-utils/flock.c lib/monotonic.c
-flock_LDADD = $(LDADD) libcommon.la $(CLOCKGETTIME_LIBS)
+flock_LDADD = $(LDADD) libcommon.la -lrt
endif
if BUILD_IPCMK
diff --git a/sys-utils/flock.c b/sys-utils/flock.c
index cb8a82b..97e7325 100644
--- a/sys-utils/flock.c
+++ b/sys-utils/flock.c
@@ -43,7 +43,6 @@
#include "nls.h"
#include "strutils.h"
#include "closestream.h"
-#include "timer.h"
#include "monotonic.h"
static void __attribute__((__noreturn__)) usage(int ex)
@@ -77,9 +76,12 @@ static void __attribute__((__noreturn__)) usage(int ex)
static sig_atomic_t timeout_expired = 0;
-static void timeout_handler(int sig __attribute__((__unused__)))
+static void timeout_handler(int sig __attribute__((__unused__)),
+ siginfo_t *info,
+ void *context __attribute__((__unused__)))
{
- timeout_expired = 1;
+ if (info->si_code == SI_TIMER)
+ timeout_expired = 1;
}
static int open_file(const char *filename, int *flags)
@@ -111,9 +113,42 @@ static int open_file(const char *filename, int *flags)
return fd;
}
+static int setup_timer(timer_t *t_id, struct itimerval *timeout)
+{
+ struct sigaction sig_a;
+ static struct sigevent sig_e = {
+ .sigev_notify = SIGEV_SIGNAL,
+ .sigev_signo = SIGALRM
+ };
+ struct itimerspec val = {
+ .it_value.tv_sec = timeout->it_value.tv_sec,
+ .it_value.tv_nsec = timeout->it_value.tv_usec * 1000,
+ .it_interval.tv_sec = 0,
+ .it_interval.tv_nsec = 0
+ };
+
+ if (sigemptyset(&sig_a.sa_mask))
+ return 1;
+ sig_a.sa_flags = SA_SIGINFO;
+ sig_a.sa_handler = (void (*)(int))timeout_handler;
+ if (sigaction(SIGALRM, &sig_a, 0))
+ return 1;
+ if (timer_create(CLOCK_MONOTONIC, &sig_e, t_id))
+ return 1;
+ if (timer_settime(*t_id, SA_SIGINFO, &val, NULL))
+ return 1;
+ return 0;
+}
+
+static void cancel_timer(timer_t *t_id)
+{
+ timer_delete(*t_id);
+}
+
int main(int argc, char *argv[])
{
- struct itimerval timeout, old_timer;
+ static timer_t t_id;
+ struct itimerval timeout;
int have_timeout = 0;
int type = LOCK_EX;
int block = 0;
@@ -131,7 +166,6 @@ int main(int argc, char *argv[])
int conflict_exit_code = 1;
char **cmd_argv = NULL, *sh_c_argv[4];
const char *filename = NULL;
- struct sigaction old_sa;
enum {
OPT_VERBOSE = CHAR_MAX + 1
};
@@ -246,7 +280,8 @@ int main(int argc, char *argv[])
have_timeout = 0;
block = LOCK_NB;
} else
- setup_timer(&timeout, &old_timer, &old_sa, timeout_handler);
+ if (setup_timer(&t_id, &timeout))
+ err(EX_OSERR, _("cannot not setup timer"));
}
if (verbose)
@@ -298,7 +333,7 @@ int main(int argc, char *argv[])
}
if (have_timeout)
- cancel_timer(&old_timer, &old_sa);
+ cancel_timer(&t_id);
if (verbose) {
struct timeval delta;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (2 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 03/16] flock: improve timeout handling Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 10:25 ` Karel Zak
2015-02-22 14:41 ` [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err() Sami Kerola
` (13 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/prlimit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sys-utils/prlimit.c b/sys-utils/prlimit.c
index ddab76e..0fa5d57 100644
--- a/sys-utils/prlimit.c
+++ b/sys-utils/prlimit.c
@@ -360,7 +360,8 @@ static void do_prlimit(struct list_head *lims)
old = &lim->rlim;
if (verbose && new) {
- printf(_("New %s limit: "), lim->desc->name);
+ printf(_("New %s limit for pid %d: "), lim->desc->name,
+ pid ? pid : getpid());
if (new->rlim_cur == RLIM_INFINITY)
printf("<%s", _("unlimited"));
else
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err()
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (3 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 10:29 ` Karel Zak
2015-02-22 14:41 ` [PATCH 06/16] tunelp: remove unnecessary preprocessor directives Sami Kerola
` (12 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/Makemodule.am | 1 +
sys-utils/tunelp.c | 22 ++++++++--------------
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
index 14d0773..c8f1c42 100644
--- a/sys-utils/Makemodule.am
+++ b/sys-utils/Makemodule.am
@@ -50,6 +50,7 @@ if BUILD_TUNELP
usrsbin_exec_PROGRAMS += tunelp
dist_man_MANS += sys-utils/tunelp.8
tunelp_SOURCES = sys-utils/tunelp.c sys-utils/lp.h
+tunelp_LDADD = $(LDADD) libcommon.la
endif
if BUILD_FSTRIM
diff --git a/sys-utils/tunelp.c b/sys-utils/tunelp.c
index b132d7a..197b093 100644
--- a/sys-utils/tunelp.c
+++ b/sys-utils/tunelp.c
@@ -74,8 +74,10 @@
#include "xalloc.h"
#include "closestream.h"
-#define EXIT_BAD_VALUE 3
-#define EXIT_LP_IO_ERR 4
+#define STRTOXX_EXIT_CODE 3
+#define EXIT_LP_IO_ERR 4
+
+#include "strutils.h"
struct command {
long op;
@@ -114,14 +116,6 @@ static void __attribute__((__noreturn__)) print_usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static long get_val(char *val)
-{
- long ret;
- if (!(sscanf(val, "%ld", &ret) == 1))
- errx(EXIT_BAD_VALUE, _("bad value"));
- return ret;
-}
-
static long get_onoff(char *val)
{
if (!strncasecmp("on", val, 2))
@@ -171,28 +165,28 @@ int main(int argc, char **argv)
break;
case 'i':
cmds->op = LPSETIRQ;
- cmds->val = get_val(optarg);
+ cmds->val = strtol_or_err(optarg, _("argument error"));
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
break;
case 't':
cmds->op = LPTIME;
- cmds->val = get_val(optarg);
+ cmds->val = strtol_or_err(optarg, _("argument error"));
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
break;
case 'c':
cmds->op = LPCHAR;
- cmds->val = get_val(optarg);
+ cmds->val = strtol_or_err(optarg, _("argument error"));
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
break;
case 'w':
cmds->op = LPWAIT;
- cmds->val = get_val(optarg);
+ cmds->val = strtol_or_err(optarg, _("argument error"));
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 06/16] tunelp: remove unnecessary preprocessor directives
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (4 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err() Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library Sami Kerola
` (11 subsequent siblings)
17 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
The lp.h included earlier in the tunelp.c has the definitions that were
checked, so these statements could have not been false and such
impossible conditions does not need to be checked.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/tunelp.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/sys-utils/tunelp.c b/sys-utils/tunelp.c
index 197b093..8c70268 100644
--- a/sys-utils/tunelp.c
+++ b/sys-utils/tunelp.c
@@ -205,7 +205,6 @@ int main(int argc, char **argv)
show_irq = 0;
}
break;
-#ifdef LPGETSTATUS
case 'o':
cmds->op = LPABORTOPEN;
cmds->val = get_onoff(optarg);
@@ -228,8 +227,6 @@ int main(int argc, char **argv)
cmds = cmds->next;
cmds->next = 0;
break;
-#endif
-#ifdef LPRESET
case 'r':
cmds->op = LPRESET;
cmds->val = 0;
@@ -237,8 +234,6 @@ int main(int argc, char **argv)
cmds = cmds->next;
cmds->next = 0;
break;
-#endif
-#ifdef LPTRUSTIRQ
case 'T':
/* Note: this will do the wrong thing on
* 2.0.36 when compiled under 2.2.x
@@ -249,7 +244,6 @@ int main(int argc, char **argv)
cmds = cmds->next;
cmds->next = 0;
break;
-#endif
case 'v':
case 'V':
printf(UTIL_LINUX_VERSION);
@@ -290,7 +284,6 @@ int main(int argc, char **argv)
cmds = cmdst;
while (cmds->next) {
-#ifdef LPGETSTATUS
if (cmds->op == LPGETSTATUS) {
status = 0xdeadbeef;
retval = ioctl(fd, LPGETSTATUS - offset, &status);
@@ -314,7 +307,6 @@ int main(int argc, char **argv)
printf("\n");
}
} else
-#endif /* LPGETSTATUS */
if (ioctl(fd, cmds->op - offset, cmds->val) < 0)
warn(_("ioctl failed"));
cmdst = cmds;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (5 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 06/16] tunelp: remove unnecessary preprocessor directives Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 17:12 ` Benno Schulenberg
2015-02-24 10:34 ` Karel Zak
2015-02-22 14:41 ` [PATCH 08/16] tunelp: use parse_switch() Sami Kerola
` (10 subsequent siblings)
17 siblings, 2 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
To allow sharing the code with other utilities.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/strutils.h | 2 ++
lib/strutils.c | 8 ++++++++
term-utils/setterm.c | 14 --------------
3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/include/strutils.h b/include/strutils.h
index 4d8463a..ee8c7cb 100644
--- a/include/strutils.h
+++ b/include/strutils.h
@@ -36,6 +36,8 @@ extern void strtotimeval_or_err(const char *str, struct timeval *tv,
extern int isdigit_string(const char *str);
+extern int parse_switch(const char *arg, const char *a, const char *b);
+
#ifndef HAVE_MEMPCPY
extern void *mempcpy(void *restrict dest, const void *restrict src, size_t n);
#endif
diff --git a/lib/strutils.c b/lib/strutils.c
index c4f9600..f6154aa 100644
--- a/lib/strutils.c
+++ b/lib/strutils.c
@@ -181,6 +181,14 @@ int isdigit_string(const char *str)
return p && p > str && !*p;
}
+int parse_switch(const char *arg, const char *a, const char *b)
+{
+ if (strcmp(arg, a) == 0)
+ return 1;
+ else if (strcmp(arg, b) == 0)
+ return 0;
+ errx(STRTOXX_EXIT_CODE, _("argument error: %s"), arg);
+}
#ifndef HAVE_MEMPCPY
void *mempcpy(void *restrict dest, const void *restrict src, size_t n)
diff --git a/term-utils/setterm.c b/term-utils/setterm.c
index 76fedba..253f84f 100644
--- a/term-utils/setterm.c
+++ b/term-utils/setterm.c
@@ -178,20 +178,6 @@ struct setterm_control {
opt_powerdown:1, opt_blength:1, opt_bfreq:1;
};
-/* Command line parsing routines.
- *
- * Note that it is an error for a given option to be invoked more than once.
- */
-
-static int parse_switch(const char *arg, const char *t, const char *f)
-{
- if (strcmp(arg, t) == 0)
- return 1;
- else if (strcmp(arg, f) == 0)
- return 0;
- errx(EXIT_FAILURE, _("argument error: %s"), arg);
-}
-
static int parse_febg_color(const char *arg)
{
int color;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 08/16] tunelp: use parse_switch()
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (6 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 10:40 ` Karel Zak
2015-02-22 14:41 ` [PATCH 09/16] eject: " Sami Kerola
` (9 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/tunelp.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/sys-utils/tunelp.c b/sys-utils/tunelp.c
index 8c70268..65c397a 100644
--- a/sys-utils/tunelp.c
+++ b/sys-utils/tunelp.c
@@ -116,13 +116,6 @@ static void __attribute__((__noreturn__)) print_usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static long get_onoff(char *val)
-{
- if (!strncasecmp("on", val, 2))
- return 1;
- return 0;
-}
-
int main(int argc, char **argv)
{
int c, fd, irq, status, show_irq, offset = 0, retval;
@@ -193,28 +186,24 @@ int main(int argc, char **argv)
break;
case 'a':
cmds->op = LPABORT;
- cmds->val = get_onoff(optarg);
+ cmds->val = parse_switch(optarg, "on", "off");
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
break;
case 'q':
- if (get_onoff(optarg)) {
- show_irq = 1;
- } else {
- show_irq = 0;
- }
+ show_irq = parse_switch(optarg, "on", "off");
break;
case 'o':
cmds->op = LPABORTOPEN;
- cmds->val = get_onoff(optarg);
+ cmds->val = parse_switch(optarg, "on", "off");
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
break;
case 'C':
cmds->op = LPCAREFUL;
- cmds->val = get_onoff(optarg);
+ cmds->val = parse_switch(optarg, "on", "off");
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
@@ -239,7 +228,7 @@ int main(int argc, char **argv)
* 2.0.36 when compiled under 2.2.x
*/
cmds->op = LPTRUSTIRQ;
- cmds->val = get_onoff(optarg);
+ cmds->val = parse_switch(optarg, "on", "off");
cmds->next = xmalloc(sizeof(struct command));
cmds = cmds->next;
cmds->next = 0;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 09/16] eject: use parse_switch()
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (7 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 08/16] tunelp: use parse_switch() Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 11:37 ` Karel Zak
2015-02-22 14:41 ` [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes Sami Kerola
` (8 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/eject.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/sys-utils/eject.c b/sys-utils/eject.c
index 4f7d6e6..4e1d65f 100644
--- a/sys-utils/eject.c
+++ b/sys-utils/eject.c
@@ -202,12 +202,7 @@ static void parse_args(struct eject_control *ctl, int argc, char **argv)
switch (c) {
case 'a':
ctl->a_option = 1;
- if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
- ctl->a_arg = 0;
- else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
- ctl->a_arg = 1;
- else
- errx(EXIT_FAILURE, _("invalid argument to --auto/-a option"));
+ ctl->a_arg = parse_switch(optarg, "on", "off");
break;
case 'c':
ctl->c_option = 1;
@@ -231,12 +226,7 @@ static void parse_args(struct eject_control *ctl, int argc, char **argv)
break;
case 'i':
ctl->i_option = 1;
- if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
- ctl->i_arg = 0;
- else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
- ctl->i_arg = 1;
- else
- errx(EXIT_FAILURE, _("invalid argument to --manualeject/-i option"));
+ ctl->i_arg = parse_switch(optarg, "on", "off");
break;
case 'm':
ctl->m_option = 1;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (8 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 09/16] eject: " Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 11:47 ` Karel Zak
2015-02-22 14:41 ` [PATCH 11/16] tailf: check printing criteria more carefully Sami Kerola
` (7 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
disk-utils/fdisk.c | 2 +-
disk-utils/fsck.minix.c | 6 +++---
disk-utils/sfdisk.c | 2 +-
include/rpmatch.h | 4 ++++
login-utils/vipw.c | 2 +-
term-utils/mesg.c | 6 +++---
6 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/disk-utils/fdisk.c b/disk-utils/fdisk.c
index 8a0bfd5..815f3f3 100644
--- a/disk-utils/fdisk.c
+++ b/disk-utils/fdisk.c
@@ -347,7 +347,7 @@ int ask_callback(struct fdisk_context *cxt, struct fdisk_ask *ask,
if (rc)
break;
x = rpmatch(buf);
- if (x == 1 || x == 0) {
+ if (x == RPMATCH_YES || x == RPMATCH_NO) {
fdisk_ask_yesno_set_result(ask, x);
break;
}
diff --git a/disk-utils/fsck.minix.c b/disk-utils/fsck.minix.c
index ac2dc47..52b001b 100644
--- a/disk-utils/fsck.minix.c
+++ b/disk-utils/fsck.minix.c
@@ -269,11 +269,11 @@ ask(const char *string, int def) {
ignore_result( fgets(input, YESNO_LENGTH, stdin) );
resp = rpmatch(input);
switch (resp) {
- case -1:
+ case RPMATCH_INVALID:
/* def = def */
break;
- case 0:
- case 1:
+ case RPMATCH_NO:
+ case RPMATCH_YES:
def = resp;
break;
default:
diff --git a/disk-utils/sfdisk.c b/disk-utils/sfdisk.c
index b7d4480..bce94e5 100644
--- a/disk-utils/sfdisk.c
+++ b/disk-utils/sfdisk.c
@@ -167,7 +167,7 @@ static int ask_callback(struct fdisk_context *cxt,
if (rc)
break;
x = rpmatch(buf);
- if (x == 1 || x == 0) {
+ if (x == RPMATCH_YES || x == RPMATCH_NO) {
fdisk_ask_yesno_set_result(ask, x);
break;
}
diff --git a/include/rpmatch.h b/include/rpmatch.h
index d62634b..f64d52e 100644
--- a/include/rpmatch.h
+++ b/include/rpmatch.h
@@ -6,4 +6,8 @@
(*r == 'y' || *r == 'Y' ? 1 : *r == 'n' || *r == 'N' ? 0 : -1)
#endif
+#define RPMATCH_YES 1
+#define RPMATCH_NO 0
+#define RPMATCH_INVALID -1
+
#endif /* UTIL_LINUX_RPMATCH_H */
diff --git a/login-utils/vipw.c b/login-utils/vipw.c
index 668f4d8..9fb2550 100644
--- a/login-utils/vipw.c
+++ b/login-utils/vipw.c
@@ -352,7 +352,7 @@ int main(int argc, char *argv[])
printf(_("Would you like to edit %s now [y/n]? "), orig_file);
if (fgets(response, sizeof(response), stdin)) {
- if (rpmatch(response) == 1)
+ if (rpmatch(response) == RPMATCH_YES)
edit_file(1);
}
}
diff --git a/term-utils/mesg.c b/term-utils/mesg.c
index 3cb1d24..77d89c7 100644
--- a/term-utils/mesg.c
+++ b/term-utils/mesg.c
@@ -137,7 +137,7 @@ int main(int argc, char *argv[])
}
switch (rpmatch(argv[0])) {
- case 1:
+ case RPMATCH_YES:
#ifdef USE_TTY_GROUP
if (chmod(tty, sb.st_mode | S_IWGRP) < 0)
#else
@@ -147,13 +147,13 @@ int main(int argc, char *argv[])
if (verbose)
puts(_("write access to your terminal is allowed"));
return IS_ALLOWED;
- case 0:
+ case RPMATCH_NO:
if (chmod(tty, sb.st_mode & ~(S_IWGRP|S_IWOTH)) < 0)
err(MESG_EXIT_FAILURE, _("change %s mode failed"), tty);
if (verbose)
puts(_("write access to your terminal is denied"));
return IS_NOT_ALLOWED;
- case -1:
+ case RPMATCH_INVALID:
warnx(_("invalid argument: %s"), argv[0]);
usage(stderr);
default:
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 11/16] tailf: check printing criteria more carefully
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (9 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 11:53 ` Karel Zak
2015-02-22 14:41 ` [PATCH 12/16] tailf: count last lines correctly at initial print out Sami Kerola
` (6 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Earlier size check did not print out contents of a file if it happen to
be same size as last time roll_file() function ran. The earlier buggy
behavior was easy to reproduce with two terminal windows. Contents of
the second echo were not visible.
term1> tailf /tmp/tailf-test
terminal2> echo 1 >| /tmp/tailf-test
terminal2> echo 2 >| /tmp/tailf-test
In this commit also a mount point and a file changes are detected,
leading to full printout of a file in such cases.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
text-utils/tailf.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/text-utils/tailf.c b/text-utils/tailf.c
index 38df99f..0c477ad 100644
--- a/text-utils/tailf.c
+++ b/text-utils/tailf.c
@@ -86,7 +86,7 @@ tailf(const char *filename, int lines)
}
static void
-roll_file(const char *filename, off_t *size)
+roll_file(const char *filename, struct stat *old)
{
char buf[BUFSIZ];
int fd;
@@ -100,12 +100,17 @@ roll_file(const char *filename, off_t *size)
if (fstat(fd, &st) == -1)
err(EXIT_FAILURE, _("stat of %s failed"), filename);
- if (st.st_size == *size) {
+ if (st.st_size == old->st_size && st.st_mtime == old->st_mtime &&
+ st.st_ino == old->st_ino && st.st_dev == old->st_dev) {
close(fd);
return;
}
- if (lseek(fd, *size, SEEK_SET) != (off_t)-1) {
+ if (st.st_ino != old->st_ino || st.st_dev != old->st_dev
+ || (old->st_mtime < st.st_mtime && st.st_size <= old->st_size))
+ old->st_size = 0;
+
+ if (lseek(fd, old->st_size, SEEK_SET) != (off_t)-1) {
ssize_t rc, wc;
while ((rc = read(fd, buf, sizeof(buf))) > 0) {
@@ -123,16 +128,16 @@ roll_file(const char *filename, off_t *size)
* avoids data duplication. If we read nothing or hit an error, reset
* to the reported size, this handles truncated files.
*/
- *size = (pos != -1 && pos != *size) ? pos : st.st_size;
+ old->st_size = (pos != -1 && pos != old->st_size) ? pos : 0;
close(fd);
}
static void
-watch_file(const char *filename, off_t *size)
+watch_file(const char *filename, struct stat *old)
{
do {
- roll_file(filename, size);
+ roll_file(filename, old);
xusleep(250000);
} while(1);
}
@@ -144,7 +149,7 @@ watch_file(const char *filename, off_t *size)
#define NEVENTS 4
static int
-watch_file_inotify(const char *filename, off_t *size)
+watch_file_inotify(const char *filename, struct stat *old)
{
char buf[ NEVENTS * sizeof(struct inotify_event) ];
int fd, ffd, e;
@@ -176,7 +181,7 @@ watch_file_inotify(const char *filename, off_t *size)
struct inotify_event *ev = (struct inotify_event *) &buf[e];
if (ev->mask & IN_MODIFY)
- roll_file(filename, size);
+ roll_file(filename, old);
else {
close(ffd);
ffd = -1;
@@ -237,8 +242,7 @@ int main(int argc, char **argv)
const char *filename;
long lines;
int ch;
- struct stat st;
- off_t size = 0;
+ struct stat old;
static const struct option longopts[] = {
{ "lines", required_argument, 0, 'n' },
@@ -277,16 +281,15 @@ int main(int argc, char **argv)
filename = argv[optind];
- if (stat(filename, &st) != 0)
+ if (stat(filename, &old) != 0)
err(EXIT_FAILURE, _("stat of %s failed"), filename);
- size = st.st_size;;
tailf(filename, lines);
#ifdef HAVE_INOTIFY_INIT
- if (!watch_file_inotify(filename, &size))
+ if (!watch_file_inotify(filename, &old))
#endif
- watch_file(filename, &size);
+ watch_file(filename, &old);
return EXIT_SUCCESS;
}
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 12/16] tailf: count last lines correctly at initial print out
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (10 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 11/16] tailf: check printing criteria more carefully Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 12:12 ` Karel Zak
2015-02-22 14:41 ` [PATCH 13/16] tailf: do not allow minus signed last lines argument Sami Kerola
` (5 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
When last lines happen to be greater than string buffer size for fgets()
the number of printed lines resulted to too few.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
text-utils/tailf.c | 56 +++++++++++++++++++++++++-----------------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/text-utils/tailf.c b/text-utils/tailf.c
index 0c477ad..b5ea4c2 100644
--- a/text-utils/tailf.c
+++ b/text-utils/tailf.c
@@ -36,6 +36,8 @@
#include <ctype.h>
#include <errno.h>
#include <getopt.h>
+#include <sys/mman.h>
+
#ifdef HAVE_INOTIFY_INIT
#include <sys/inotify.h>
#endif
@@ -49,40 +51,34 @@
#define DEFAULT_LINES 10
static void
-tailf(const char *filename, int lines)
+tailf(const char *filename, unsigned long lines, struct stat *old)
{
- char *buf, *p;
- int head = 0;
- int tail = 0;
- FILE *str;
- int i;
+ int fd;
+ size_t i;
+ char *data;
- if (!(str = fopen(filename, "r")))
+ if (!(fd = open(filename, O_RDONLY)))
err(EXIT_FAILURE, _("cannot open %s"), filename);
-
- buf = xmalloc((lines ? lines : 1) * BUFSIZ);
- p = buf;
- while (fgets(p, BUFSIZ, str)) {
- if (++tail >= lines) {
- tail = 0;
- head = 1;
+ data = mmap(0, old->st_size, PROT_READ, MAP_SHARED, fd, 0);
+ i = (size_t)old->st_size - 1;
+ /* humans do not think last new line in a file should be counted,
+ * in that case do off by one from counter point of view */
+ if (data[i] == '\n')
+ lines++;
+ while (i) {
+ if (data[i] == '\n') {
+ if (--lines == 0) {
+ i++;
+ break;
+ }
}
- p = buf + (tail * BUFSIZ);
+ i--;
}
-
- if (head) {
- for (i = tail; i < lines; i++)
- fputs(buf + (i * BUFSIZ), stdout);
- for (i = 0; i < tail; i++)
- fputs(buf + (i * BUFSIZ), stdout);
- } else {
- for (i = head; i < tail; i++)
- fputs(buf + (i * BUFSIZ), stdout);
- }
-
+ while (i < (size_t)old->st_size)
+ putchar(data[i++]);
+ munmap(data, old->st_size);
+ close(fd);
fflush(stdout);
- free(buf);
- fclose(str);
}
static void
@@ -283,8 +279,8 @@ int main(int argc, char **argv)
if (stat(filename, &old) != 0)
err(EXIT_FAILURE, _("stat of %s failed"), filename);
-
- tailf(filename, lines);
+ if (lines && old.st_size)
+ tailf(filename, lines, &old);
#ifdef HAVE_INOTIFY_INIT
if (!watch_file_inotify(filename, &old))
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 13/16] tailf: do not allow minus signed last lines argument
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (11 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 12/16] tailf: count last lines correctly at initial print out Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 14/16] tailf: ensure file argument really is a file Sami Kerola
` (4 subsequent siblings)
17 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Before mmap() the command behavior was not completely correct, as
demonstrated below, and after the mmap() it tried to print some eighteen
quintillion lines.
$ tailf -n-1 x
tailf: cannot allocate 18446744073709543424 bytes: Cannot allocate memory
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
text-utils/tailf.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/text-utils/tailf.c b/text-utils/tailf.c
index b5ea4c2..d856203 100644
--- a/text-utils/tailf.c
+++ b/text-utils/tailf.c
@@ -213,16 +213,16 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
}
/* parses -N option */
-static long old_style_option(int *argc, char **argv)
+static long old_style_option(int *argc, char **argv, unsigned long *lines)
{
- int i = 1, nargs = *argc;
- long lines = -1;
+ int i = 1, nargs = *argc, ret = 0;
while(i < nargs) {
if (argv[i][0] == '-' && isdigit(argv[i][1])) {
- lines = strtol_or_err(argv[i] + 1,
+ *lines = strtoul_or_err(argv[i] + 1,
_("failed to parse number of lines"));
nargs--;
+ ret = 1;
if (nargs - i)
memmove(argv + i, argv + i + 1,
sizeof(char *) * (nargs - i));
@@ -230,13 +230,13 @@ static long old_style_option(int *argc, char **argv)
i++;
}
*argc = nargs;
- return lines;
+ return ret;
}
int main(int argc, char **argv)
{
const char *filename;
- long lines;
+ unsigned long lines;
int ch;
struct stat old;
@@ -252,16 +252,18 @@ int main(int argc, char **argv)
textdomain(PACKAGE);
atexit(close_stdout);
- lines = old_style_option(&argc, argv);
- if (lines < 0)
+ if (!old_style_option(&argc, argv, &lines))
lines = DEFAULT_LINES;
while ((ch = getopt_long(argc, argv, "n:N:Vh", longopts, NULL)) != -1)
- switch((char)ch) {
+ switch ((char)ch) {
case 'n':
case 'N':
- lines = strtol_or_err(optarg,
- _("failed to parse number of lines"));
+ if (optarg[0] == '-')
+ errx(EXIT_FAILURE, "%s: %s",
+ _("failed to parse number of lines"), optarg);
+ lines =
+ strtoul_or_err(optarg, _("failed to parse number of lines"));
break;
case 'V':
printf(UTIL_LINUX_VERSION);
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 14/16] tailf: ensure file argument really is a file
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (12 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 13/16] tailf: do not allow minus signed last lines argument Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 12:18 ` Karel Zak
2015-02-22 14:41 ` [PATCH 15/16] logger: move /dev/log to pathnames.h Sami Kerola
` (3 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
The tailf(1) never worked very well with block or character devices,
sockets, fifos and such. Now after mmap() is used to find last lines
even the little command used to work is broken, so test the tailf is
asked to follow a file and when not fail. That said symlinks are OK, as
long they point to a file.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
text-utils/tailf.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/text-utils/tailf.c b/text-utils/tailf.c
index d856203..7f053a4 100644
--- a/text-utils/tailf.c
+++ b/text-utils/tailf.c
@@ -233,6 +233,28 @@ static long old_style_option(int *argc, char **argv, unsigned long *lines)
return ret;
}
+static int is_file(const char *filename, const struct stat sb)
+{
+ switch (sb.st_mode & S_IFMT) {
+ case S_IFREG:
+ return 0;
+ case S_IFLNK:
+ {
+ char *resolved_path = NULL;
+ struct stat follow;
+ int ret;
+
+ if (realpath(filename, resolved_path)) {
+ stat(resolved_path, &follow);
+ ret = is_file(resolved_path, follow);
+ free(resolved_path);
+ return ret;
+ }
+ }
+ }
+ return 1;
+}
+
int main(int argc, char **argv)
{
const char *filename;
@@ -281,6 +303,8 @@ int main(int argc, char **argv)
if (stat(filename, &old) != 0)
err(EXIT_FAILURE, _("stat of %s failed"), filename);
+ if (is_file(filename, old))
+ errx(EXIT_FAILURE, _("%s: is not a file"), filename);
if (lines && old.st_size)
tailf(filename, lines, &old);
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 15/16] logger: move /dev/log to pathnames.h
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (13 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 14/16] tailf: ensure file argument really is a file Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-24 12:19 ` Karel Zak
2015-02-22 14:41 ` [PATCH 16/16] logger: fix -i argument parsing regression Sami Kerola
` (2 subsequent siblings)
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
include/pathnames.h | 3 +++
misc-utils/logger.c | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/pathnames.h b/include/pathnames.h
index cbc93b7..cc01589 100644
--- a/include/pathnames.h
+++ b/include/pathnames.h
@@ -193,5 +193,8 @@
/* kernel command line */
#define _PATH_PROC_CMDLINE "/proc/cmdline"
+/* logger paths */
+#define _PATH_DEVLOG "/dev/log"
+
#endif /* PATHNAMES_H */
diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index a3af7f1..db6fd44 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -56,6 +56,7 @@
#include "c.h"
#include "closestream.h"
#include "nls.h"
+#include "pathnames.h"
#include "strutils.h"
#include "xalloc.h"
@@ -481,7 +482,7 @@ static void logger_open(struct logger_ctl *ctl)
ctl->syslogfp = syslog_rfc5424;
return;
}
- ctl->fd = unix_socket(ctl, "/dev/log", ctl->socket_type);
+ ctl->fd = unix_socket(ctl, _PATH_DEVLOG, ctl->socket_type);
ctl->syslogfp = syslog_local;
}
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 16/16] logger: fix -i argument parsing regression
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (14 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 15/16] logger: move /dev/log to pathnames.h Sami Kerola
@ 2015-02-22 14:41 ` Sami Kerola
2015-02-22 17:21 ` Benno Schulenberg
2015-02-22 17:10 ` [PATCH 11/62] pull: Benno Schulenberg
2015-03-05 10:16 ` [PATCH 00/16] pull: changes to various utils Karel Zak
17 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-02-22 14:41 UTC (permalink / raw
To: util-linux; +Cc: Sami Kerola
With earlier logger it's possible to combine the option -i with other
options, such as -s. But currently:
$:~> logger -is
logger: failed to parse id: 's'
The changed behaviour breaks existing scripts like dhcpcd-run-hooks from
dhcpcd.
Broken-since: aab5b44405b9a6ada92e419e5a84cc0d1d4afee9
Reference: http://comments.gmane.org/gmane.linux.utilities.util-linux-ng/9683
Reported-by: Juergen Daubert <jue@jue.li>
Reviewed-by: Benno Schulenberg <benno@vertaalt.nl>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
misc-utils/logger.1 | 5 ++++-
misc-utils/logger.c | 13 +++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/misc-utils/logger.1 b/misc-utils/logger.1
index a855cdf..1bf8f2e 100644
--- a/misc-utils/logger.1
+++ b/misc-utils/logger.1
@@ -58,7 +58,10 @@ syslog port defined in /etc/services, which is often 514 .
Log the contents of the specified \fIfile\fR.
This option cannot be combined with a command-line message.
.TP
-.BR \-i , " \-\-id" [ = \fIid ]
+.BR \-i
+Log the PID of the logger process with each line.
+.TP
+.BR "\-\-id" [ = \fIid ]
Log the PID of the logger process with each line. When the optional
argument \fIid\fR is specified, then it is used instead of the logger
command's PID. The use of \fB\-\-id=$$\fR
diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index db6fd44..9f9af7e 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -89,7 +89,8 @@ enum {
OPT_JOURNALD,
OPT_RFC3164,
OPT_RFC5424,
- OPT_SOCKET_ERRORS
+ OPT_SOCKET_ERRORS,
+ OPT_ID
};
struct logger_ctl {
@@ -548,7 +549,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
fputs(_("Enter messages into the system log.\n"), out);
fputs(USAGE_OPTIONS, out);
- fputs(_(" -i, --id[=<id>] log <id> (default is PID)\n"), out);
+ fputs(_(" -i log logger command PID\n"), out);
+ fputs(_(" --id[=<id>] log <id> (default is PID)\n"), out);
fputs(_(" -f, --file <file> log the contents of this file\n"), out);
fputs(_(" -p, --priority <prio> mark given message with this priority\n"), out);
fputs(_(" --prio-prefix look for a prefix on every line read from stdin\n"), out);
@@ -606,7 +608,7 @@ int main(int argc, char **argv)
FILE *jfd = NULL;
#endif
static const struct option longopts[] = {
- { "id", optional_argument, 0, 'i' },
+ { "id", optional_argument, 0, OPT_ID },
{ "stderr", no_argument, 0, 's' },
{ "file", required_argument, 0, 'f' },
{ "priority", required_argument, 0, 'p' },
@@ -633,7 +635,7 @@ int main(int argc, char **argv)
textdomain(PACKAGE);
atexit(close_stdout);
- while ((ch = getopt_long(argc, argv, "f:i::p:st:u:dTn:P:Vh",
+ while ((ch = getopt_long(argc, argv, "f:ip:st:u:dTn:P:Vh",
longopts, NULL)) != -1) {
switch (ch) {
case 'f': /* file to log */
@@ -642,6 +644,9 @@ int main(int argc, char **argv)
stdout_reopened = 1;
break;
case 'i': /* log process id also */
+ ctl.pid = getpid();
+ break;
+ case OPT_ID:
if (optarg) {
const char *p = optarg;
--
2.3.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 11/62] pull: ...
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (15 preceding siblings ...)
2015-02-22 14:41 ` [PATCH 16/16] logger: fix -i argument parsing regression Sami Kerola
@ 2015-02-22 17:10 ` Benno Schulenberg
2015-03-05 10:16 ` [PATCH 00/16] pull: changes to various utils Karel Zak
17 siblings, 0 replies; 48+ messages in thread
From: Benno Schulenberg @ 2015-02-22 17:10 UTC (permalink / raw
To: Sami Kerola; +Cc: Util-Linux
On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> Here is collection of patches [...]
Oh mayday, mayday, m... Where's the panic button?
:)
Benno
--
http://www.fastmail.com - Send your email first class
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library
2015-02-22 14:41 ` [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library Sami Kerola
@ 2015-02-22 17:12 ` Benno Schulenberg
2015-02-24 10:34 ` Karel Zak
1 sibling, 0 replies; 48+ messages in thread
From: Benno Schulenberg @ 2015-02-22 17:12 UTC (permalink / raw
To: Sami Kerola; +Cc: Util-Linux
On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> -/* Command line parsing routines.
> - *
When removing this comment, please also remove its companion:
/* End of command line parsing routines. */
(It would have been needed anyway, since the usage() routine has
intruded.)
Benno
--
http://www.fastmail.com - Access your email from home and the web
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 16/16] logger: fix -i argument parsing regression
2015-02-22 14:41 ` [PATCH 16/16] logger: fix -i argument parsing regression Sami Kerola
@ 2015-02-22 17:21 ` Benno Schulenberg
2015-03-01 11:06 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Benno Schulenberg @ 2015-02-22 17:21 UTC (permalink / raw
To: Sami Kerola; +Cc: Util-Linux
On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> Reviewed-by: Benno Schulenberg <benno@vertaalt.nl>
Please use my justemail address (the current one) here instead.
The vertaalt one I use just for translations.
> +.BR \-i
s/BR/B/ (since there is just one argument)
> +.BR "\-\-id" [ = \fIid ]
s/= /=/ (to fix my own mistake)
> - fputs(_(" -i, --id[=<id>] log <id> (default is PID)\n"), out);
> + fputs(_(" -i log logger command PID\n"), out);
Better: "log the logger command's PID" -- there is room enough to
avoid telegram style.
> + fputs(_(" --id[=<id>] log <id> (default is PID)\n"), out);
Better: "log the given <id>, or otherwise the PID"
Benno
--
http://www.fastmail.com - Access all of your messages and folders
wherever you are
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
@ 2015-02-22 17:37 ` Benno Schulenberg
2015-02-24 9:40 ` Karel Zak
1 sibling, 0 replies; 48+ messages in thread
From: Benno Schulenberg @ 2015-02-22 17:37 UTC (permalink / raw
To: Sami Kerola; +Cc: Util-Linux
On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> misc-utils/whereis.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
But whereis works fine with a missing -f when some
other option is given after -B or -M or -S. Example:
$ whereis -B /crap -bm when where whereis
when:
where:
whereis: /usr/share/man/man1/whereis.1.gz
To break that with a complaint about a missing -f...
Better not.
But when there's no intervening option, then it's good
to report a missing -f -- at least, when there is more
than one argument after the -B or -M or -S.
> + if (opt_f_missing)
> + errx(EXIT_FAILURE, _("option -f missing"));
s/missing/is missing/
Benno
--
http://www.fastmail.com - Send your email first class
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 02/16] flock: add --verbose option
2015-02-22 14:41 ` [PATCH 02/16] flock: add --verbose option Sami Kerola
@ 2015-02-22 17:44 ` Benno Schulenberg
2015-03-01 11:11 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Benno Schulenberg @ 2015-02-22 17:44 UTC (permalink / raw
To: Sami Kerola; +Cc: Util-Linux
On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> +.B \-\-verbose
> +Tell why lock could not be got, and when it can be how long did it take
> +to acquire the lock.
Better: "Report how long it took to acquire the lock, or why the lock
could not be obtained."
> - {"version", no_argument, NULL, 'V'},
> + {"version", required_argument, NULL, 'V'},
Huh?
Benno
--
http://www.fastmail.com - mmm... Fastmail...
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
2015-02-22 17:37 ` Benno Schulenberg
@ 2015-02-24 9:40 ` Karel Zak
2015-03-01 11:08 ` Sami Kerola
1 sibling, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 9:40 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:31PM +0000, Sami Kerola wrote:
> misc-utils/whereis.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
Benno is right, see construct_dirlist_from_argv(), the dir list is
terminated by arbitrary option, it checks for '-'.
The expected use-case is "<dirlist> <search-restriction> <pattern>",
for example:
whereis -M /usr/share/man/man1 -m ls
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] flock: improve timeout handling
2015-02-22 14:41 ` [PATCH 03/16] flock: improve timeout handling Sami Kerola
@ 2015-02-24 10:21 ` Karel Zak
2015-03-01 11:46 ` Sami Kerola
2015-03-05 9:36 ` Karel Zak
1 sibling, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 10:21 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:33PM +0000, Sami Kerola wrote:
> Signal ALRM raised by the timer, and the timer only, will be considered
> as a timeout criteria.
>
> Secondly time interval is made to use monotonic clock. Documentation of
> ITIMER_REAL is unclear whether that time is affected various sources of
> clock skew, or does it even tick when system is suspended.
I agree, setitimer() API is also obsolete.
> This code is moved from libcommon.la to flock.c because of two reasons.
> This is the only utility using the function, and setup_timer() along with
> cancel_timer() need to be linked with -lrt option.
Hmm... yes, it's used by flock only, but I still think it would be
better to keep it in common lib/ code. Maybe we can use the timers on
another places later (uuidd, login, sulogin, ...).
What about to keep all -lrt stuff in lib/monotonic.c?
> --- a/sys-utils/Makemodule.am
> +++ b/sys-utils/Makemodule.am
> @@ -2,7 +2,7 @@ if BUILD_FLOCK
> usrbin_exec_PROGRAMS += flock
> dist_man_MANS += sys-utils/flock.1
> flock_SOURCES = sys-utils/flock.c lib/monotonic.c
> -flock_LDADD = $(LDADD) libcommon.la $(CLOCKGETTIME_LIBS)
> +flock_LDADD = $(LDADD) libcommon.la -lrt
Don't use -l<foo> in Makefiles, always use $(FOO) and initialize the
variable in ./configure, $(CLOCKGETTIME_LIBS) is fine (although the
name of the variable is not perfect in this context).
> +static void timeout_handler(int sig __attribute__((__unused__)),
> + siginfo_t *info,
> + void *context __attribute__((__unused__)))
> {
> - timeout_expired = 1;
> + if (info->si_code == SI_TIMER)
> + timeout_expired = 1;
> }
BTW, it mean that "kill -ALRM" does not force the program to set
timeout_expired, right? Nice.
> +static int setup_timer(timer_t *t_id, struct itimerval *timeout)
if you move it to lib/ than add a pointer to timeout_handler() too
Anyway, it seems more elegant than the previous implementation.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit
2015-02-22 14:41 ` [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit Sami Kerola
@ 2015-02-24 10:25 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 10:25 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:34PM +0000, Sami Kerola wrote:
> sys-utils/prlimit.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err()
2015-02-22 14:41 ` [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err() Sami Kerola
@ 2015-02-24 10:29 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 10:29 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:35PM +0000, Sami Kerola wrote:
> sys-utils/Makemodule.am | 1 +
> sys-utils/tunelp.c | 22 ++++++++--------------
> 2 files changed, 9 insertions(+), 14 deletions(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library
2015-02-22 14:41 ` [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library Sami Kerola
2015-02-22 17:12 ` Benno Schulenberg
@ 2015-02-24 10:34 ` Karel Zak
1 sibling, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 10:34 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:37PM +0000, Sami Kerola wrote:
> include/strutils.h | 2 ++
> lib/strutils.c | 8 ++++++++
> term-utils/setterm.c | 14 --------------
> 3 files changed, 10 insertions(+), 14 deletions(-)
Applied (and removed the extra comment as suggested by Benno), thanks.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 08/16] tunelp: use parse_switch()
2015-02-22 14:41 ` [PATCH 08/16] tunelp: use parse_switch() Sami Kerola
@ 2015-02-24 10:40 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 10:40 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:38PM +0000, Sami Kerola wrote:
> sys-utils/tunelp.c | 21 +++++----------------
> 1 file changed, 5 insertions(+), 16 deletions(-)
Applied, thanks.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 09/16] eject: use parse_switch()
2015-02-22 14:41 ` [PATCH 09/16] eject: " Sami Kerola
@ 2015-02-24 11:37 ` Karel Zak
2015-03-01 11:49 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 11:37 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:39PM +0000, Sami Kerola wrote:
> case 'a':
> ctl->a_option = 1;
> - if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
> - ctl->a_arg = 0;
> - else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
> - ctl->a_arg = 1;
> - else
> - errx(EXIT_FAILURE, _("invalid argument to --auto/-a option"));
> + ctl->a_arg = parse_switch(optarg, "on", "off");
I have extended parse_switch() to accept arbitrary number of 1|0 pairs
and also error message, so:
ctl->a_arg = parse_switch(optarg, _("argument error"),
"on", "off", "1", "0", NULL);
Idea: maybe we can remove all the _("argument error") strings from all
strtoxxx_ functions and use some global function to generate proper user
friendly error messages from longopts[], something like:
while ((c = getopt_long(argc, argv, "a:b:", longopts, NULL)) != -1) {
err_strtoxxx_set_options(c, longopts); <--- !!!
switch (c) {
case 'a':
foo = strtol_or_err(optarg);
break;
case 'b':
bar = parse_switch(optarg, "on", "off", NULL);
break;
}
}
where err_strtoxxx_set_options() sets in strutils.c static
ul_cur_option = c;
ul_lonfopts = longopts;
then on error strtol_or_err() and parse_switch() will use the ul_
variables to print
"invalid argument to --auto/-a: %s", optarg
and if ul_* are not initialized then something generic ("argument
error"). The result will be less code in main(), better error messages,
and code independent on the names of the options.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes
2015-02-22 14:41 ` [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes Sami Kerola
@ 2015-02-24 11:47 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 11:47 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:40PM +0000, Sami Kerola wrote:
> disk-utils/fdisk.c | 2 +-
> disk-utils/fsck.minix.c | 6 +++---
> disk-utils/sfdisk.c | 2 +-
> include/rpmatch.h | 4 ++++
> login-utils/vipw.c | 2 +-
> term-utils/mesg.c | 6 +++---
> 6 files changed, 13 insertions(+), 9 deletions(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/16] tailf: check printing criteria more carefully
2015-02-22 14:41 ` [PATCH 11/16] tailf: check printing criteria more carefully Sami Kerola
@ 2015-02-24 11:53 ` Karel Zak
2015-03-01 11:55 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 11:53 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:41PM +0000, Sami Kerola wrote:
> - if (lseek(fd, *size, SEEK_SET) != (off_t)-1) {
> + if (st.st_ino != old->st_ino || st.st_dev != old->st_dev
> + || (old->st_mtime < st.st_mtime && st.st_size <= old->st_size))
> + old->st_size = 0;
So.. if I remove (truncate) last line from 1GiB file then tailf will
print all the file *again*? Would be better to reset to st.st_size if the
size is smaller than old->st_size?
Or maybe if you want to support "echo 1 >| file" use case than add a
new option (--rewind) or so.
> + if (lseek(fd, old->st_size, SEEK_SET) != (off_t)-1) {
> ssize_t rc, wc;
>
> while ((rc = read(fd, buf, sizeof(buf))) > 0) {
> @@ -123,16 +128,16 @@ roll_file(const char *filename, off_t *size)
> * avoids data duplication. If we read nothing or hit an error, reset
> * to the reported size, this handles truncated files.
> */
> - *size = (pos != -1 && pos != *size) ? pos : st.st_size;
> + old->st_size = (pos != -1 && pos != old->st_size) ? pos : 0;
This way how to reset seems better.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 12/16] tailf: count last lines correctly at initial print out
2015-02-22 14:41 ` [PATCH 12/16] tailf: count last lines correctly at initial print out Sami Kerola
@ 2015-02-24 12:12 ` Karel Zak
2015-03-01 12:09 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 12:12 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:42PM +0000, Sami Kerola wrote:
> When last lines happen to be greater than string buffer size for fgets()
> the number of printed lines resulted to too few.
Maybe add a note about mmap() to commit message ?!
> - if (!(str = fopen(filename, "r")))
> + if (!(fd = open(filename, O_RDONLY)))
> err(EXIT_FAILURE, _("cannot open %s"), filename);
> -
> - buf = xmalloc((lines ? lines : 1) * BUFSIZ);
> - p = buf;
> - while (fgets(p, BUFSIZ, str)) {
> - if (++tail >= lines) {
> - tail = 0;
> - head = 1;
> + data = mmap(0, old->st_size, PROT_READ, MAP_SHARED, fd, 0);
It would be nice to compare this it with original implementation on
large files. I guess mmap will be more effective and faster.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 14/16] tailf: ensure file argument really is a file
2015-02-22 14:41 ` [PATCH 14/16] tailf: ensure file argument really is a file Sami Kerola
@ 2015-02-24 12:18 ` Karel Zak
2015-03-01 12:20 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-02-24 12:18 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:44PM +0000, Sami Kerola wrote:
> text-utils/tailf.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/text-utils/tailf.c b/text-utils/tailf.c
> index d856203..7f053a4 100644
> --- a/text-utils/tailf.c
> +++ b/text-utils/tailf.c
> @@ -233,6 +233,28 @@ static long old_style_option(int *argc, char **argv, unsigned long *lines)
> return ret;
> }
>
> +static int is_file(const char *filename, const struct stat sb)
> +{
> + switch (sb.st_mode & S_IFMT) {
> + case S_IFREG:
> + return 0;
> + case S_IFLNK:
> + {
> + char *resolved_path = NULL;
> + struct stat follow;
> + int ret;
> +
> + if (realpath(filename, resolved_path)) {
> + stat(resolved_path, &follow);
> + ret = is_file(resolved_path, follow);
> + free(resolved_path);
> + return ret;
> + }
> + }
but you use stat() and no lstat(), so you don't have to care about
symlinks, right? :-)
> if (stat(filename, &old) != 0)
> err(EXIT_FAILURE, _("stat of %s failed"), filename);
if (IS_REG(old.st_mode))
errx(EXIT_FAILURE, _("%s: is not a file"), filename);
> + if (is_file(filename, old))
> + errx(EXIT_FAILURE, _("%s: is not a file"), filename);
> if (lines && old.st_size)
> tailf(filename, lines, &old);
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 15/16] logger: move /dev/log to pathnames.h
2015-02-22 14:41 ` [PATCH 15/16] logger: move /dev/log to pathnames.h Sami Kerola
@ 2015-02-24 12:19 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-02-24 12:19 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:45PM +0000, Sami Kerola wrote:
> include/pathnames.h | 3 +++
> misc-utils/logger.c | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 16/16] logger: fix -i argument parsing regression
2015-02-22 17:21 ` Benno Schulenberg
@ 2015-03-01 11:06 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:06 UTC (permalink / raw
To: Benno Schulenberg; +Cc: Util-Linux
On Sun, 22 Feb 2015, Benno Schulenberg wrote:
> On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> > Reviewed-by: Benno Schulenberg <benno@vertaalt.nl>
>
> Please use my justemail address (the current one) here instead.
> The vertaalt one I use just for translations.
>
> > +.BR \-i
>
> s/BR/B/ (since there is just one argument)
>
> > +.BR "\-\-id" [ = \fIid ]
>
> s/= /=/ (to fix my own mistake)
>
> > - fputs(_(" -i, --id[=<id>] log <id> (default is PID)\n"), out);
> > + fputs(_(" -i log logger command PID\n"), out);
>
> Better: "log the logger command's PID" -- there is room enough to
> avoid telegram style.
Nalime Benno,
> > + fputs(_(" --id[=<id>] log <id> (default is PID)\n"), out);
>
> Better: "log the given <id>, or otherwise the PID"
Here is updated version of the change.
https://github.com/kerolasa/lelux-utiliteetit/commit/7dbe2082a9d87316d7aed7d1c8a7f94a8a80b1f0
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-02-24 9:40 ` Karel Zak
@ 2015-03-01 11:08 ` Sami Kerola
2015-03-03 11:32 ` Karel Zak
0 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:08 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:31PM +0000, Sami Kerola wrote:
> > misc-utils/whereis.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
>
> Benno is right, see construct_dirlist_from_argv(), the dir list is
> terminated by arbitrary option, it checks for '-'.
>
> The expected use-case is "<dirlist> <search-restriction> <pattern>",
> for example:
>
> whereis -M /usr/share/man/man1 -m ls
Hi Benno & Karel,
I see, most of the options has to flick the opt_f_missing. Here is updated
version of the change.
https://github.com/kerolasa/lelux-utiliteetit/commit/ada8f7f0b253a031936519f7953629d701832444
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 02/16] flock: add --verbose option
2015-02-22 17:44 ` Benno Schulenberg
@ 2015-03-01 11:11 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:11 UTC (permalink / raw
To: Benno Schulenberg; +Cc: Util-Linux
On Sun, 22 Feb 2015, Benno Schulenberg wrote:
> On Sun, Feb 22, 2015, at 15:41, Sami Kerola wrote:
> > +.B \-\-verbose
> > +Tell why lock could not be got, and when it can be how long did it take
> > +to acquire the lock.
>
> Better: "Report how long it took to acquire the lock, or why the lock
> could not be obtained."
>
> > - {"version", no_argument, NULL, 'V'},
> > + {"version", required_argument, NULL, 'V'},
>
> Huh?
Thanks Benno,
Wording is updated, and --version option argument reuirement reverted. To
be honest I have no idea how that error slipped to change, excellent you
noticed it.
https://github.com/kerolasa/lelux-utiliteetit/commit/f1cb67206af5e6b46ba10b482d3c315c484082d6
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] flock: improve timeout handling
2015-02-24 10:21 ` Karel Zak
@ 2015-03-01 11:46 ` Sami Kerola
2015-03-02 9:13 ` Karel Zak
0 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:46 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:33PM +0000, Sami Kerola wrote:
> > Signal ALRM raised by the timer, and the timer only, will be considered
> > as a timeout criteria.
> >
> > Secondly time interval is made to use monotonic clock. Documentation of
> > ITIMER_REAL is unclear whether that time is affected various sources of
> > clock skew, or does it even tick when system is suspended.
>
> I agree, setitimer() API is also obsolete.
>
> > This code is moved from libcommon.la to flock.c because of two reasons.
> > This is the only utility using the function, and setup_timer() along with
> > cancel_timer() need to be linked with -lrt option.
>
> Hmm... yes, it's used by flock only, but I still think it would be
> better to keep it in common lib/ code. Maybe we can use the timers on
> another places later (uuidd, login, sulogin, ...).
>
> What about to keep all -lrt stuff in lib/monotonic.c?
Thanks Karel,
Moving to monotonic sounds good to me. Notice that there is linking change
in configure.ac making monotonic.c to be linked with -lrt even when
clock_gettime() would not require it. The timer_create() requires -lrt at
last for now.
https://github.com/kerolasa/lelux-utiliteetit/commit/50425160a825986abb4e9c22ef8acee8e7b34834
> > --- a/sys-utils/Makemodule.am
> > +++ b/sys-utils/Makemodule.am
> > @@ -2,7 +2,7 @@ if BUILD_FLOCK
> > usrbin_exec_PROGRAMS += flock
> > dist_man_MANS += sys-utils/flock.1
> > flock_SOURCES = sys-utils/flock.c lib/monotonic.c
> > -flock_LDADD = $(LDADD) libcommon.la $(CLOCKGETTIME_LIBS)
> > +flock_LDADD = $(LDADD) libcommon.la -lrt
>
> Don't use -l<foo> in Makefiles, always use $(FOO) and initialize the
> variable in ./configure, $(CLOCKGETTIME_LIBS) is fine (although the
> name of the variable is not perfect in this context).
TODO:
s/CLOCKGETTIME_LIBS/UL_REALTIME_LIBS/
Better?
> > +static void timeout_handler(int sig __attribute__((__unused__)),
> > + siginfo_t *info,
> > + void *context __attribute__((__unused__)))
> > {
> > - timeout_expired = 1;
> > + if (info->si_code == SI_TIMER)
> > + timeout_expired = 1;
> > }
>
> BTW, it mean that "kill -ALRM" does not force the program to set
> timeout_expired, right? Nice.
Correct. After the change flock(1) cannot be forced to timeout by sending
an ALRM signal. Is that good, bad, or possible regression is debatable. I
think allowing external signals to cause timeout is unintented behavior.
If someone requires external timeouts then adding an option to allow them
is justified.
> > +static int setup_timer(timer_t *t_id, struct itimerval *timeout)
>
> if you move it to lib/ than add a pointer to timeout_handler() too
>
> Anyway, it seems more elegant than the previous implementation.
Function pointer is added.
https://github.com/kerolasa/lelux-utiliteetit/commit/50425160a825986abb4e9c22ef8acee8e7b34834
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 09/16] eject: use parse_switch()
2015-02-24 11:37 ` Karel Zak
@ 2015-03-01 11:49 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:49 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:39PM +0000, Sami Kerola wrote:
> > case 'a':
> > ctl->a_option = 1;
> > - if (!strcmp(optarg, "0") || !strcmp(optarg, "off"))
> > - ctl->a_arg = 0;
> > - else if (!strcmp(optarg, "1") || !strcmp(optarg, "on"))
> > - ctl->a_arg = 1;
> > - else
> > - errx(EXIT_FAILURE, _("invalid argument to --auto/-a option"));
> > + ctl->a_arg = parse_switch(optarg, "on", "off");
>
> I have extended parse_switch() to accept arbitrary number of 1|0 pairs
> and also error message, so:
>
> ctl->a_arg = parse_switch(optarg, _("argument error"),
> "on", "off", "1", "0", NULL);
>
>
> Idea: maybe we can remove all the _("argument error") strings from all
> strtoxxx_ functions and use some global function to generate proper user
> friendly error messages from longopts[], something like:
>
>
> while ((c = getopt_long(argc, argv, "a:b:", longopts, NULL)) != -1) {
>
> err_strtoxxx_set_options(c, longopts); <--- !!!
>
> switch (c) {
> case 'a':
> foo = strtol_or_err(optarg);
> break;
> case 'b':
> bar = parse_switch(optarg, "on", "off", NULL);
> break;
> }
> }
>
>
> where err_strtoxxx_set_options() sets in strutils.c static
>
> ul_cur_option = c;
> ul_lonfopts = longopts;
>
> then on error strtol_or_err() and parse_switch() will use the ul_
> variables to print
>
> "invalid argument to --auto/-a: %s", optarg
>
> and if ul_* are not initialized then something generic ("argument
> error"). The result will be less code in main(), better error messages,
> and code independent on the names of the options.
Thanks Karel,
Updated version of parse_switch() is now in use in 'tunelp: use
parse_switch()' change.
https://github.com/kerolasa/lelux-utiliteetit/commit/d5ea22b18085c8a16621bc2e2cf45200afe3fe9c
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 11/16] tailf: check printing criteria more carefully
2015-02-24 11:53 ` Karel Zak
@ 2015-03-01 11:55 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 11:55 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:41PM +0000, Sami Kerola wrote:
> > - if (lseek(fd, *size, SEEK_SET) != (off_t)-1) {
> > + if (st.st_ino != old->st_ino || st.st_dev != old->st_dev
> > + || (old->st_mtime < st.st_mtime && st.st_size <= old->st_size))
> > + old->st_size = 0;
>
> So.. if I remove (truncate) last line from 1GiB file then tailf will
> print all the file *again*? Would be better to reset to st.st_size if the
> size is smaller than old->st_size?
>
> Or maybe if you want to support "echo 1 >| file" use case than add a
> new option (--rewind) or so.
Thanks Karel,
The 1GiB file last line truncation is such strong argument against the
change that I removed it from my remote branch. Maybe the tailf.1 manual
page should mention truncations are not handled always correctly, and
that's as good the command is ever going to be.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 12/16] tailf: count last lines correctly at initial print out
2015-02-24 12:12 ` Karel Zak
@ 2015-03-01 12:09 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 12:09 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:42PM +0000, Sami Kerola wrote:
> > When last lines happen to be greater than string buffer size for fgets()
> > the number of printed lines resulted to too few.
>
> Maybe add a note about mmap() to commit message ?!
Thanks Karel,
Done.
https://github.com/kerolasa/lelux-utiliteetit/commit/537f3b197f3c6af307997ebd103becbf44f529c1
> > - if (!(str = fopen(filename, "r")))
> > + if (!(fd = open(filename, O_RDONLY)))
> > err(EXIT_FAILURE, _("cannot open %s"), filename);
> > -
> > - buf = xmalloc((lines ? lines : 1) * BUFSIZ);
> > - p = buf;
> > - while (fgets(p, BUFSIZ, str)) {
> > - if (++tail >= lines) {
> > - tail = 0;
> > - head = 1;
> > + data = mmap(0, old->st_size, PROT_READ, MAP_SHARED, fd, 0);
>
> It would be nice to compare this it with original implementation on
> large files. I guess mmap will be more effective and faster.
My motivation to do this change had less to do with speed than correcness.
Earlier version did not print requested number or lines when tail read
buffer was insufficient. That probably did not happen too often as 8K is a
lot of text.
The reason I chose mmap() is simple. It provides easy interface to read a
file backwards. And a potential speed gain is a nice side effect.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 14/16] tailf: ensure file argument really is a file
2015-02-24 12:18 ` Karel Zak
@ 2015-03-01 12:20 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-01 12:20 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On Tue, 24 Feb 2015, Karel Zak wrote:
> On Sun, Feb 22, 2015 at 02:41:44PM +0000, Sami Kerola wrote:
> > text-utils/tailf.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/text-utils/tailf.c b/text-utils/tailf.c
> > index d856203..7f053a4 100644
> > --- a/text-utils/tailf.c
> > +++ b/text-utils/tailf.c
> > @@ -233,6 +233,28 @@ static long old_style_option(int *argc, char **argv, unsigned long *lines)
> > return ret;
> > }
> >
> > +static int is_file(const char *filename, const struct stat sb)
> > +{
> > + switch (sb.st_mode & S_IFMT) {
> > + case S_IFREG:
> > + return 0;
> > + case S_IFLNK:
> > + {
> > + char *resolved_path = NULL;
> > + struct stat follow;
> > + int ret;
> > +
> > + if (realpath(filename, resolved_path)) {
> > + stat(resolved_path, &follow);
> > + ret = is_file(resolved_path, follow);
> > + free(resolved_path);
> > + return ret;
> > + }
> > + }
>
> but you use stat() and no lstat(), so you don't have to care about
> symlinks, right? :-)
>
> > if (stat(filename, &old) != 0)
> > err(EXIT_FAILURE, _("stat of %s failed"), filename);
>
> if (IS_REG(old.st_mode))
> errx(EXIT_FAILURE, _("%s: is not a file"), filename);
Thanks Karel,
Change is made a lot nicer, and I found what got side lined;
((st.st_mode & S_IFMT) == S_IFREG) != S_ISREG(st.st_mode)
Let's hope I will never forget that in future.
https://github.com/kerolasa/lelux-utiliteetit/commit/a7a3440221e74408ef7727b6c3c83ed68e6e4936
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] flock: improve timeout handling
2015-03-01 11:46 ` Sami Kerola
@ 2015-03-02 9:13 ` Karel Zak
0 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-03-02 9:13 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 01, 2015 at 11:46:11AM +0000, Sami Kerola wrote:
> Moving to monotonic sounds good to me. Notice that there is linking change
> in configure.ac making monotonic.c to be linked with -lrt even when
> clock_gettime() would not require it. The timer_create() requires -lrt at
> last for now.
>
> https://github.com/kerolasa/lelux-utiliteetit/commit/50425160a825986abb4e9c22ef8acee8e7b34834
OK.
> s/CLOCKGETTIME_LIBS/UL_REALTIME_LIBS/
>
> Better?
Yeah.
> Correct. After the change flock(1) cannot be forced to timeout by sending
> an ALRM signal. Is that good, bad, or possible regression is debatable. I
> think allowing external signals to cause timeout is unintented behavior.
I think it's better to split signals and timers.
> Function pointer is added.
>
> https://github.com/kerolasa/lelux-utiliteetit/commit/50425160a825986abb4e9c22ef8acee8e7b34834
You forgot to update commit message ("This code is moved ...").
Anyway, looks good.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-03-01 11:08 ` Sami Kerola
@ 2015-03-03 11:32 ` Karel Zak
2015-03-03 12:59 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Karel Zak @ 2015-03-03 11:32 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Mar 01, 2015 at 11:08:29AM +0000, Sami Kerola wrote:
> On Tue, 24 Feb 2015, Karel Zak wrote:
>
> > On Sun, Feb 22, 2015 at 02:41:31PM +0000, Sami Kerola wrote:
> > > misc-utils/whereis.c | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > Benno is right, see construct_dirlist_from_argv(), the dir list is
> > terminated by arbitrary option, it checks for '-'.
> >
> > The expected use-case is "<dirlist> <search-restriction> <pattern>",
> > for example:
> >
> > whereis -M /usr/share/man/man1 -m ls
>
> Hi Benno & Karel,
>
> I see, most of the options has to flick the opt_f_missing. Here is updated
> version of the change.
>
> https://github.com/kerolasa/lelux-utiliteetit/commit/ada8f7f0b253a031936519f7953629d701832444
Did you update your "various" branch? I still see old patches on
github. (And I'd like to "git pull" :-)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-03-03 11:32 ` Karel Zak
@ 2015-03-03 12:59 ` Sami Kerola
2015-03-03 23:07 ` Sami Kerola
0 siblings, 1 reply; 48+ messages in thread
From: Sami Kerola @ 2015-03-03 12:59 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On 3 March 2015 at 11:32, Karel Zak <kzak@redhat.com> wrote:
> Did you update your "various" branch? I still see old patches on
> github. (And I'd like to "git pull" :-)
I left the original branches as-is. Now when all has had enough time
to have look of my github commits I'll rewrite the history where
needed, and send update pull as various2, script2, more2, and so on
branches. Since that's not a lot of work I assume to do the submission
at evening (work is distracting hobbies).
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 01/16] whereis: tell when mandatory option is missing
2015-03-03 12:59 ` Sami Kerola
@ 2015-03-03 23:07 ` Sami Kerola
0 siblings, 0 replies; 48+ messages in thread
From: Sami Kerola @ 2015-03-03 23:07 UTC (permalink / raw
To: Karel Zak; +Cc: util-linux
On 3 March 2015 at 12:59, Sami Kerola <kerolasa@iki.fi> wrote:
> On 3 March 2015 at 11:32, Karel Zak <kzak@redhat.com> wrote:
>> Did you update your "various" branch? I still see old patches on
>> github. (And I'd like to "git pull" :-)
>
> I left the original branches as-is. Now when all has had enough time
> to have look of my github commits I'll rewrite the history where
> needed, and send update pull as various2, script2, more2, and so on
> branches. Since that's not a lot of work I assume to do the submission
> at evening (work is distracting hobbies).
The second versions of the four branches are available.
Fix to comment Benno mentioned is here. And of course I removed the
earlier mistake addition to unrelated commit.
https://github.com/kerolasa/lelux-utiliteetit/commit/fc5097c31b7e5eb43e92e577b2b66c6fe66de240
Karel, your proposal to avoid unnecessary fopen() && fclose() pairs
resulted to new commit
https://github.com/kerolasa/lelux-utiliteetit/commit/2a6f4ec60281bf69f1ac2d72d06752c376b78053
that is also below, but does not make the script2 branch quite ready.
The HAVE_LIBUTEMPTER problem is still present.
Summary: the following two branches ought to be ready for merge:
git://github.com/kerolasa/lelux-utiliteetit.git rtcwake2
git://github.com/kerolasa/lelux-utiliteetit.git various2
p.s. Here's mail list review copy of commit 2a6f4ec6.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Tue, 3 Mar 2015 22:06:01 +0000
Subject: [PATCH] script: move timing file opening close to use of it
This allows removing almost immediate closure of file handle in the
doshell() function that does not use the file.
Proposed-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
term-utils/script.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/term-utils/script.c b/term-utils/script.c
index e15ea38..6812548 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -90,6 +90,7 @@ struct script_control {
char *cflg; /* command to be executed */
char *fname; /* output file path */
FILE *typescriptfp; /* output file pointer */
+ char *tname; /* timing file path */
FILE *timingfp; /* timing file pointer */
struct timeval oldtime; /* previous write or command start time */
int master; /* pseudoterminal master file descriptor */
@@ -169,6 +170,9 @@ static void __attribute__((__noreturn__))
done(struct script_control *ctl)
else
exit(WEXITSTATUS(ctl->childstatus));
}
+ if (ctl->timingfp)
+ fclose(ctl->timingfp);
+ fclose(ctl->typescriptfp);
exit(EXIT_SUCCESS);
}
@@ -271,8 +275,17 @@ static void do_io(struct script_control *ctl)
time_t tvec = time((time_t *)NULL);
char buf[128];
- if (ctl->tflg && !ctl->timingfp)
- ctl->timingfp = fdopen(STDERR_FILENO, "w");
+ if ((ctl->typescriptfp = fopen(ctl->fname, ctl->aflg ? "a" :
"w")) == NULL) {
+ warn(_("cannot open %s"), ctl->fname);
+ fail(ctl);
+ }
+ if (ctl->tflg) {
+ if (!ctl->tname) {
+ if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
+ err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
+ } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
+ err(EXIT_FAILURE, _("cannot open %s"), ctl->tname);
+ }
pfd[0].fd = STDIN_FILENO;
pfd[0].events = POLLIN;
@@ -344,11 +357,6 @@ static void __attribute__((__noreturn__))
doshell(struct script_control *ctl)
/* close things irrelevant for this process */
close(ctl->master);
- if (ctl->typescriptfp)
- fclose(ctl->typescriptfp);
- if (ctl->timingfp)
- fclose(ctl->timingfp);
- ctl->typescriptfp = ctl->timingfp = NULL;
dup2(ctl->slave, STDIN_FILENO);
dup2(ctl->slave, STDOUT_FILENO);
@@ -522,8 +530,8 @@ int main(int argc, char **argv)
ctl.qflg = 1;
break;
case 't':
- if (optarg && !(ctl.timingfp = fopen(optarg, "w")))
- err(EXIT_FAILURE, _("cannot open %s"), optarg);
+ if (optarg)
+ ctl.tname = optarg;
ctl.tflg = 1;
break;
case 'V':
@@ -546,11 +554,6 @@ int main(int argc, char **argv)
die_if_link(&ctl);
}
- if ((ctl.typescriptfp = fopen(ctl.fname, ctl.aflg ? "a" : "w")) == NULL) {
- warn(_("cannot open %s"), ctl.fname);
- fail(&ctl);
- }
-
ctl.shell = getenv("SHELL");
if (ctl.shell == NULL)
ctl.shell = _PATH_BSHELL;
--
2.3.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 03/16] flock: improve timeout handling
2015-02-22 14:41 ` [PATCH 03/16] flock: improve timeout handling Sami Kerola
2015-02-24 10:21 ` Karel Zak
@ 2015-03-05 9:36 ` Karel Zak
1 sibling, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-03-05 9:36 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:33PM +0000, Sami Kerola wrote:
> + sig_a.sa_flags = SA_SIGINFO;
> + sig_a.sa_handler = (void (*)(int))timeout_handler;
man sigaction:
If SA_SIGINFO is specified in sa_flags, then sa_sigaction (instead of
sa_handler) specifies the signal-handling function for signum.
I'll fix it...
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 00/16] pull: changes to various utils
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
` (16 preceding siblings ...)
2015-02-22 17:10 ` [PATCH 11/62] pull: Benno Schulenberg
@ 2015-03-05 10:16 ` Karel Zak
17 siblings, 0 replies; 48+ messages in thread
From: Karel Zak @ 2015-03-05 10:16 UTC (permalink / raw
To: Sami Kerola; +Cc: util-linux
On Sun, Feb 22, 2015 at 02:41:30PM +0000, Sami Kerola wrote:
> git://github.com/kerolasa/lelux-utiliteetit.git various
Merged (various2), thanks.
Look forward to see regression tests for flock, logger and whereis.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2015-03-05 10:17 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-22 14:41 [PATCH 00/16] pull: changes to various utils Sami Kerola
2015-02-22 14:41 ` [PATCH 01/16] whereis: tell when mandatory option is missing Sami Kerola
2015-02-22 17:37 ` Benno Schulenberg
2015-02-24 9:40 ` Karel Zak
2015-03-01 11:08 ` Sami Kerola
2015-03-03 11:32 ` Karel Zak
2015-03-03 12:59 ` Sami Kerola
2015-03-03 23:07 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 02/16] flock: add --verbose option Sami Kerola
2015-02-22 17:44 ` Benno Schulenberg
2015-03-01 11:11 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 03/16] flock: improve timeout handling Sami Kerola
2015-02-24 10:21 ` Karel Zak
2015-03-01 11:46 ` Sami Kerola
2015-03-02 9:13 ` Karel Zak
2015-03-05 9:36 ` Karel Zak
2015-02-22 14:41 ` [PATCH 04/16] prlimit: tell in --verbose output which pid got the new limit Sami Kerola
2015-02-24 10:25 ` Karel Zak
2015-02-22 14:41 ` [PATCH 05/16] tunelp: remove get_val() in favour of strtol_or_err() Sami Kerola
2015-02-24 10:29 ` Karel Zak
2015-02-22 14:41 ` [PATCH 06/16] tunelp: remove unnecessary preprocessor directives Sami Kerola
2015-02-22 14:41 ` [PATCH 07/16] lib/strutils: move parse_switch() from setterm(1) to library Sami Kerola
2015-02-22 17:12 ` Benno Schulenberg
2015-02-24 10:34 ` Karel Zak
2015-02-22 14:41 ` [PATCH 08/16] tunelp: use parse_switch() Sami Kerola
2015-02-24 10:40 ` Karel Zak
2015-02-22 14:41 ` [PATCH 09/16] eject: " Sami Kerola
2015-02-24 11:37 ` Karel Zak
2015-03-01 11:49 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 10/16] rpmatch: use symbolic value when evaluation return codes Sami Kerola
2015-02-24 11:47 ` Karel Zak
2015-02-22 14:41 ` [PATCH 11/16] tailf: check printing criteria more carefully Sami Kerola
2015-02-24 11:53 ` Karel Zak
2015-03-01 11:55 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 12/16] tailf: count last lines correctly at initial print out Sami Kerola
2015-02-24 12:12 ` Karel Zak
2015-03-01 12:09 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 13/16] tailf: do not allow minus signed last lines argument Sami Kerola
2015-02-22 14:41 ` [PATCH 14/16] tailf: ensure file argument really is a file Sami Kerola
2015-02-24 12:18 ` Karel Zak
2015-03-01 12:20 ` Sami Kerola
2015-02-22 14:41 ` [PATCH 15/16] logger: move /dev/log to pathnames.h Sami Kerola
2015-02-24 12:19 ` Karel Zak
2015-02-22 14:41 ` [PATCH 16/16] logger: fix -i argument parsing regression Sami Kerola
2015-02-22 17:21 ` Benno Schulenberg
2015-03-01 11:06 ` Sami Kerola
2015-02-22 17:10 ` [PATCH 11/62] pull: Benno Schulenberg
2015-03-05 10:16 ` [PATCH 00/16] pull: changes to various utils Karel Zak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.