See one more comment in the code.
Vita
On Mon, Sep 03, 2012 at 02:06:13PM +0200, Nikola Pajkovsky wrote:
> Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com>
> ---
> src/iptraf.c | 43 ++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/src/iptraf.c b/src/iptraf.c
> index 15789a9..9fe7138 100644
> --- a/src/iptraf.c
> +++ b/src/iptraf.c
> @@ -34,6 +34,10 @@ An IP Network Statistics Utility
> #define WITHALL 1
> #define WITHOUTALL 0
>
> +#ifndef IPTRAF_PIDFILE
> +#define IPTRAF_PIDFILE "/var/run/iptraf-ng.pid"
> +#endif
> +
Could you, please, make it configurable from the command line?
> const char *ALLSPEC = "all";
>
> #define CMD(name, h) { .cmd = #name, .fn = cmd_##name, .help = h }
> @@ -283,6 +287,29 @@ static struct options iptraf_ng_options[] = {
> OPT_END()
> };
>
> +static int create_pidfile(void)
> +{
> + int fd = open(IPTRAF_PIDFILE, O_WRONLY|O_CREAT, 0644);
> + if (fd < 0) {
> + perror("can not open "IPTRAF_PIDFILE);
> + return -1;
> + }
> +
> + if (lockf(fd, F_TLOCK, 0) < 0) {
> + perror("can not lock file "IPTRAF_PIDFILE);
> + return -1;
> + }
^^^^
Thease are not much descriptive errors printed to user whe ne tries to
launch one more instance of iptraf-ng. Either be quiet here in
create_pidfile() and print error after call to this func or print more
descriptive error message here in create_pidfile().
Vita
> +
> + fcntl(fd, F_SETFD, FD_CLOEXEC);
> +
> + char buf[sizeof(long) * 3 + 2];
> + int len = sprintf(buf, "%lu\n", (long) getpid());
> + write(fd, buf, len);
> + ftruncate(fd, len);
> + /* we leak opened+locked fd intentionally */
> + return 0;
^^^^^^^^
change this to tabs!
> +}
> +
> static void sanitize_dir(const char *dir)
> {
> /* Check whether LOCKDIR exists (/var/run is on a tmpfs in Ubuntu) */
> @@ -397,6 +424,12 @@ int main(int argc, char **argv)
>
> loadoptions();
>
The next part should go somewhere to the line 371 after this one:
if (help_opt)
parse_usage_and_die(iptraf_ng_usage, iptraf_ng_options);
> +
> + if (create_pidfile() < 0)
> + goto cleanup;
This is before ncurses initialization. Should only remove pidfile.
> +
> + int pidfile_created = 1;
> +
> /*
> * If a facility is directly invoked from the command line, check for
> * a daemonization request
> @@ -414,9 +447,10 @@ int main(int argc, char **argv)
> options.logging = 1;
> break;
> case -1: /* error */
> - die("Fork error, %s cannot run in background", IPTRAF_NAME);
> + error("Fork error, %s cannot run in background", IPTRAF_NAME);
> + goto cleanup;
> default: /* parent */
> - exit(0);
> + goto cleanup;
> }
> }
>
> @@ -496,10 +530,13 @@ int main(int argc, char **argv)
> else
> program_interface();
>
This cleanup should go after ncurses finalization.
> +cleanup:
> + if (pidfile_created)
> + unlink(IPTRAF_PIDFILE);
> erase();
> update_panels();
> doupdate();
> endwin();
Here.
>
> - return (0);
> + return 0;
> }
> --
> 1.7.11.5