From 8bc573c3f96bbe102fba3e65769b442542c1d0b4 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Fri, 25 Jan 2013 15:27:52 +0000 Subject: [PATCH 01/10] Output errno string if socket functions fail. --- vitunes.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vitunes.c b/vitunes.c index 7d1abbb..5949586 100644 --- a/vitunes.c +++ b/vitunes.c @@ -130,7 +130,7 @@ main(int argc, char *argv[]) printf("Vitunes appears to be running already. Won't open socket."); } else { if((sock = sock_listen()) == -1) - errx(1, "failed to open socket."); + err(1, "failed to open socket"); } @@ -476,8 +476,9 @@ handle_switches(int argc, char *argv[]) switch (ch) { case 'c': if(sock_send_msg(optarg) == -1) - errx(1, "Failed to send message. Vitunes not running?"); - exit(0); + err(1, "Failed to send message. Vitunes not running?"); + had_c_commands = 1; + break; case 'd': free(db_file); From 2b0cc7360964f59de10f7ff18a02363519822c31 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Fri, 25 Jan 2013 15:34:18 +0000 Subject: [PATCH 02/10] Plug file descriptor leak. --- socket.c | 1 + 1 file changed, 1 insertion(+) diff --git a/socket.c b/socket.c index af4dcfb..b4996e9 100644 --- a/socket.c +++ b/socket.c @@ -46,6 +46,7 @@ sock_send_msg(const char *msg) return -1; } + close(ret); return 0; } From 86b337d64a93f88613d8b72c4c94f659b8e0ed82 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Fri, 25 Jan 2013 15:35:46 +0000 Subject: [PATCH 03/10] Display this on stderr, instead. --- vitunes.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vitunes.c b/vitunes.c index 5949586..97c0298 100644 --- a/vitunes.c +++ b/vitunes.c @@ -126,9 +126,9 @@ main(int argc, char *argv[]) /* handle command-line switches & e-commands */ handle_switches(argc, argv); - if(sock_send_msg(VITUNES_RUNNING) != -1) { - printf("Vitunes appears to be running already. Won't open socket."); - } else { + if(sock_send_msg(VITUNES_RUNNING) != -1) + warnx("Vitunes appears to be running already. Won't open socket."); + else { if((sock = sock_listen()) == -1) err(1, "failed to open socket"); } From ace4347f178e05936734b1452297be924f4ec2b3 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sat, 26 Jan 2013 14:46:04 +0000 Subject: [PATCH 04/10] Display informational messages on stdout. --- medialib.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/medialib.c b/medialib.c index 35cfd2e..d256263 100644 --- a/medialib.c +++ b/medialib.c @@ -159,7 +159,7 @@ medialib_setup_files(const char *vitunes_dir, const char *db_file, else err(1, "unable to create vitunes directory '%s'", vitunes_dir); } else - warnx("vitunes directory '%s' created", vitunes_dir); + printf("vitunes directory '%s' created\n", vitunes_dir); /* create playlists directory */ if (mkdir(playlist_dir, S_IRWXU) == -1) { @@ -168,7 +168,7 @@ medialib_setup_files(const char *vitunes_dir, const char *db_file, else err(1, "unable to create playlists directory '%s'", playlist_dir); } else - warnx("playlists directory '%s' created", playlist_dir); + printf("playlists directory '%s' created\n", playlist_dir); /* create database file */ if (stat(db_file, &sb) < 0) { @@ -185,7 +185,7 @@ medialib_setup_files(const char *vitunes_dir, const char *db_file, fwrite("vitunes", strlen("vitunes"), 1, f); fwrite(version, sizeof(version), 1, f); - warnx("empty database at '%s' created", db_file); + printf("empty database at '%s' created\n", db_file); fclose(f); } else err(1, "database file '%s' exists, but cannot access it", db_file); From 9d6368fa7dbcc246856035c778c1a02ddca81eb1 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sat, 26 Jan 2013 15:05:08 +0000 Subject: [PATCH 05/10] Sort getopt(3) option characters. --- vitunes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vitunes.c b/vitunes.c index 97c0298..47d0723 100644 --- a/vitunes.c +++ b/vitunes.c @@ -472,7 +472,7 @@ handle_switches(int argc, char *argv[]) { int ch; - while ((ch = getopt(argc, argv, "he:f:d:p:m:c:")) != -1) { + while ((ch = getopt(argc, argv, "hc:d:e:f:m:p:")) != -1) { switch (ch) { case 'c': if(sock_send_msg(optarg) == -1) From b3ee3e86b5b98c526ba4d41a1fc16f781a7fb027 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sat, 26 Jan 2013 15:14:25 +0000 Subject: [PATCH 06/10] Beautify usage. By adding missing options, alphabetically sort options and wording consistency. --- doc/vitunes.1 | 11 ++++++----- vitunes.c | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/doc/vitunes.1 b/doc/vitunes.1 index 7495bef..853a484 100644 --- a/doc/vitunes.1 +++ b/doc/vitunes.1 @@ -21,9 +21,10 @@ .Sh SYNOPSIS .Nm vitunes .Bk -words -.Op Fl c Ar command +.Op Fl h +.Op Fl c Ar command Op ... .Op Fl d Ar database-file -.Op Fl e Ar command Op argument ... +.Op Fl e Ar e-command Op flags .Op Fl f Ar config-file .Op Fl m Ar media-backend .Op Fl p Ar playlist-dir @@ -42,9 +43,9 @@ play the media it indexes (via .Nm accepts the following command line options: .Bl -tag -width Fl -.It Fl c Ar command +.It Fl c Ar command Op ... Execute the specified -.Ar commands +.Ar command in the currently running .Nm instance, and exit. @@ -79,7 +80,7 @@ be specified before the e-command. .Pp The default location is .Pa ~/.vitunes/vitunes.db . -.It Fl e Ar command Ar options +.It Fl e Ar e-command Op flags Execute one of the available e-commands to manipulate the database that .Nm uses. diff --git a/vitunes.c b/vitunes.c index 47d0723..9ab07e3 100644 --- a/vitunes.c +++ b/vitunes.c @@ -264,10 +264,10 @@ main(int argc, char *argv[]) void usage(void) { - fprintf(stderr,"\ -usage: %s [-f config-file] [-d database-file] [-p playlist-dir] [-m player-path] [-e COMMAND ...]\n\ -See \"%s -e help\" for information about what e-commands are available.\n\ -", + fprintf(stderr, "\ +usage: %s [-h] [-c command [...]] [-d database-file] [-e e-command [flags]]\n\ +\t[-f config-file] [-m media-backend] [-p playlist-dir]\n\ +See \"%s -e help\" for information about what e-commands are available.\n", progname, progname); exit(1); } From 50d58189b1cb17ac1c8629c1b7d51ef3f9e4ba19 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sun, 27 Jan 2013 19:24:26 +0000 Subject: [PATCH 07/10] Don't put a space before a closing parentheses. --- doc/vitunes.1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/vitunes.1 b/doc/vitunes.1 index 853a484..b62fbac 100644 --- a/doc/vitunes.1 +++ b/doc/vitunes.1 @@ -38,7 +38,7 @@ creation/management. It is not intended to be a feature-rich media player, but rather a quick, vi-like media indexer and playlist manager that also happens to be able to play the media it indexes (via -.Xr mplayer 1 ). +.Xr mplayer 1 Ns ). .Pp .Nm accepts the following command line options: @@ -242,7 +242,7 @@ Below is a listing of all run-time commands supported by .Pp All commands are entered by typing ':' followed by the command name and any parameters (just like in -.Xr vi 1 ). +.Xr vi 1 Ns ). .Pp Note that abbreviations are also supported. That is, entering any non-ambiguous abbreviation of a command name will also From fd06f4d209a6c1a5192b2352531b14b166948caf Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sun, 27 Jan 2013 19:53:51 +0000 Subject: [PATCH 08/10] Move socket to the vitunes directory. Before, it wouldn't be possible for two simultaneous users to start vitunes and send messages through the socket. Its path (defaults to ~/.vitunes/vitunes.sock), like the remaining ones, can also be configured with the newly added -s command-line option. --- doc/vitunes.1 | 14 ++++++++++---- socket.c | 17 +++++++---------- socket.h | 10 +++++----- vitunes.c | 23 ++++++++++++++++------- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/doc/vitunes.1 b/doc/vitunes.1 index b62fbac..e057f84 100644 --- a/doc/vitunes.1 +++ b/doc/vitunes.1 @@ -28,6 +28,7 @@ .Op Fl f Ar config-file .Op Fl m Ar media-backend .Op Fl p Ar playlist-dir +.Op Fl s Ar socket-file .Ek .Sh DESCRIPTION .Nm @@ -64,9 +65,8 @@ section below. .Pp Note that for this to work, when .Nm -starts up it attempts to create a socket at -.Pa /tmp/.vitunes -that are used by this option to communicate with the original instance. +starts up it attempts to create a socket that is used by this option to +communicate with the original instance. If this socket cannot be created for any reason, this option will not work. .It Fl d Ar database-file Specifies the @@ -139,6 +139,12 @@ will be created here. .Pp The default location is .Pa ~/.vitunes/playlists/ . +.It Fl s Ar socket-file +Specifies an alternative +.Ar socket-file . +.Pp +The default location is +.Pa ~/.vitunes/vitunes.sock . .El .Sh GETTING STARTED .Nm @@ -1025,7 +1031,7 @@ Default playlist directory This can be overridden with the .Op Fl p Ar playlist-dir flag. -.It Pa /tmp/.vitunes +.It Pa ~/.vitunes/vitunes.sock Default location for the socket created on start-up that can be used to control .Nm . .It Pa /usr/local/bin/mplayer diff --git a/socket.c b/socket.c index b4996e9..d74a8b9 100644 --- a/socket.c +++ b/socket.c @@ -23,23 +23,20 @@ #include "socket.h" #include "commands.h" -#define VITUNES_SOCK "/tmp/.vitunes" - int -sock_send_msg(const char *msg) +sock_send_msg(const char *path, const char *msg) { int ret; struct sockaddr_un addr; socklen_t addr_len; - if((ret = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) return -1; addr.sun_family = AF_UNIX; - strcpy(addr.sun_path, VITUNES_SOCK); - addr_len = sizeof(addr.sun_family) + strlen(VITUNES_SOCK) + 1; + strcpy(addr.sun_path, path); + addr_len = sizeof(addr.sun_family) + strlen(path) + 1; if(sendto(ret, msg, strlen(msg), 0, (struct sockaddr *) &addr, addr_len) == -1) { close(ret); @@ -52,21 +49,21 @@ sock_send_msg(const char *msg) int -sock_listen(void) +sock_listen(const char *path) { int ret; struct sockaddr_un addr; socklen_t addr_len; int coe = 1; - unlink(VITUNES_SOCK); + unlink(path); if((ret = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) return -1; addr.sun_family = AF_UNIX; - strcpy(addr.sun_path, VITUNES_SOCK); - addr_len = sizeof(addr.sun_family) + strlen(VITUNES_SOCK) + 1; + strcpy(addr.sun_path, path); + addr_len = sizeof(addr.sun_family) + strlen(path) + 1; if(bind(ret, (struct sockaddr *) &addr, addr_len) == -1) return -1; diff --git a/socket.h b/socket.h index 155f3f0..b35fcab 100644 --- a/socket.h +++ b/socket.h @@ -21,16 +21,16 @@ #define VITUNES_RUNNING "WHOWASPHONE?" /* - * send (null terminated) msg to vitunes. Returns 0 on success, + * send (null terminated) msg to vitunes through path. Returns 0 on success, * -1 on errors. */ -int sock_send_msg(const char *msg); +int sock_send_msg(const char *path, const char *msg); /* - * open vitunes socket for listening. Returns a socket on success, - * -1 on errors. + * open vitunes socket (provided by path) for listening. Returns a socket + * on success, -1 on errors. */ -int sock_listen(void); +int sock_listen(const char *path); /* * Receive message from sock into msg. A maximum number of msg_len diff --git a/vitunes.c b/vitunes.c index 9ab07e3..8c9bf89 100644 --- a/vitunes.c +++ b/vitunes.c @@ -56,6 +56,7 @@ const char *VITUNES_DIR_FMT = "%s/.vitunes"; const char *CONF_FILE_FMT = "%s/.vitunes/vitunes.conf"; const char *DB_FILE_FMT = "%s/.vitunes/vitunes.db"; const char *PLAYLIST_DIR_FMT = "%s/.vitunes/playlists"; +const char *SOCKET_FILE_FMT = "%s/.vitunes/vitunes.sock"; /* absolute paths of key stuff */ char *vitunes_dir; @@ -63,6 +64,7 @@ char *conf_file; char *db_file; char *playlist_dir; char *player_backend; +char *socket_file; /* program name with directories removed */ char *progname; @@ -122,14 +124,16 @@ main(int argc, char *argv[]) err(1, "main: asprintf failed"); if (asprintf(&player_backend, "%s", DEFAULT_PLAYER_BACKEND) == -1) err(1, "main: asprintf failed"); + if (asprintf(&socket_file, SOCKET_FILE_FMT, home) == -1) + err(1, "main: asprintf failed"); /* handle command-line switches & e-commands */ handle_switches(argc, argv); - if(sock_send_msg(VITUNES_RUNNING) != -1) + if (sock_send_msg(socket_file, VITUNES_RUNNING) != -1) warnx("Vitunes appears to be running already. Won't open socket."); else { - if((sock = sock_listen()) == -1) + if ((sock = sock_listen(socket_file)) == -1) err(1, "failed to open socket"); } @@ -266,7 +270,7 @@ usage(void) { fprintf(stderr, "\ usage: %s [-h] [-c command [...]] [-d database-file] [-e e-command [flags]]\n\ -\t[-f config-file] [-m media-backend] [-p playlist-dir]\n\ +\t[-f config-file] [-m media-backend] [-p playlist-dir] [-s socket-file]\n\ See \"%s -e help\" for information about what e-commands are available.\n", progname, progname); exit(1); @@ -472,13 +476,12 @@ handle_switches(int argc, char *argv[]) { int ch; - while ((ch = getopt(argc, argv, "hc:d:e:f:m:p:")) != -1) { + while ((ch = getopt(argc, argv, "hc:d:e:f:m:p:s:")) != -1) { switch (ch) { case 'c': - if(sock_send_msg(optarg) == -1) + if (sock_send_msg(socket_file, optarg) == -1) err(1, "Failed to send message. Vitunes not running?"); - had_c_commands = 1; - break; + exit(0); case 'd': free(db_file); @@ -511,6 +514,12 @@ handle_switches(int argc, char *argv[]) err(1, "handle_switches: strdup playlist_dir failed"); break; + case 's': + free(socket_file); + if ((socket_file = strdup(optarg)) == NULL) + err(1, "handle_switches: strdup socket_file failed"); + break; + case 'h': case '?': default: From a1e2ab5226c15024e888cbc6f87bbc2311f5f93c Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sun, 27 Jan 2013 20:14:41 +0000 Subject: [PATCH 09/10] Always use sizeof with the socket functions. In preference of, manually calculating it or possibly using SUN_LEN. All cases are valid, because sun_path must be NUL terminated. --- socket.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/socket.c b/socket.c index d74a8b9..e7b1b97 100644 --- a/socket.c +++ b/socket.c @@ -29,16 +29,14 @@ sock_send_msg(const char *path, const char *msg) { int ret; struct sockaddr_un addr; - socklen_t addr_len; if((ret = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) return -1; addr.sun_family = AF_UNIX; strcpy(addr.sun_path, path); - addr_len = sizeof(addr.sun_family) + strlen(path) + 1; - if(sendto(ret, msg, strlen(msg), 0, (struct sockaddr *) &addr, addr_len) == -1) { + if (sendto(ret, msg, strlen(msg), 0, (struct sockaddr *) &addr, sizeof addr) == -1) { close(ret); return -1; } @@ -53,7 +51,6 @@ sock_listen(const char *path) { int ret; struct sockaddr_un addr; - socklen_t addr_len; int coe = 1; unlink(path); @@ -63,9 +60,8 @@ sock_listen(const char *path) addr.sun_family = AF_UNIX; strcpy(addr.sun_path, path); - addr_len = sizeof(addr.sun_family) + strlen(path) + 1; - if(bind(ret, (struct sockaddr *) &addr, addr_len) == -1) + if (bind(ret, (struct sockaddr *) &addr, sizeof addr) == -1) return -1; fcntl(ret, F_SETFD, FD_CLOEXEC, &coe); @@ -78,7 +74,7 @@ ssize_t sock_recv_msg(int sock, char *msg, size_t msg_len) { struct sockaddr_un addr; - socklen_t addr_len = sizeof(struct sockaddr_un); + socklen_t addr_len = sizeof addr; ssize_t ret; ret = recvfrom(sock, msg, msg_len, 0, (struct sockaddr *) &addr, &addr_len); From a0020bf36c7d5660a0861d5b3662eface30413f2 Mon Sep 17 00:00:00 2001 From: Tiago Cunha Date: Sun, 27 Jan 2013 20:19:30 +0000 Subject: [PATCH 10/10] Use strlcpy(3) to copy the socket path. Imperative, now that the path (POSIX says its size is undefined) can be user controlled. In case of truncation, set errno accordingly, which is caught by the caller. --- socket.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/socket.c b/socket.c index e7b1b97..1d9c25b 100644 --- a/socket.c +++ b/socket.c @@ -13,6 +13,8 @@ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#include #include #include #include @@ -34,7 +36,10 @@ sock_send_msg(const char *path, const char *msg) return -1; addr.sun_family = AF_UNIX; - strcpy(addr.sun_path, path); + if (strlcpy(addr.sun_path, path, sizeof addr.sun_path) >= sizeof addr.sun_path) { + errno = ENAMETOOLONG; + return -1; + } if (sendto(ret, msg, strlen(msg), 0, (struct sockaddr *) &addr, sizeof addr) == -1) { close(ret); @@ -59,7 +64,10 @@ sock_listen(const char *path) return -1; addr.sun_family = AF_UNIX; - strcpy(addr.sun_path, path); + if (strlcpy(addr.sun_path, path, sizeof addr.sun_path) >= sizeof addr.sun_path) { + errno = ENAMETOOLONG; + return -1; + } if (bind(ret, (struct sockaddr *) &addr, sizeof addr) == -1) return -1;