On Thu, May 05, 2016 at 11:42:45AM +0200, Karel Zak wrote:
> On Fri, Apr 22, 2016 at 01:21:36PM +0200, Werner Fink wrote:
> > +static int open_un_socket_and_connect(void)
> > +{
> > + struct sockaddr_un su = { /* The abstract UNIX socket of plymouth */
> > + .sun_family = AF_UNIX,
> > + .sun_path = PLYMOUTH_SOCKET_PATH,
> > + };
> > + const int one = 1;
> > + int fd, ret;
> > +
> > + fd = socket(PF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0);
> > + if (fd < 0) {
> > + warnx(_("can not open UNIX socket"));
> > + goto err;
> > + }
> > +
> > + ret = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, (socklen_t)sizeof(one));
> > + if (ret < 0) {
> > + warnx(_("can not set option for UNIX socket"));
> > + close(fd);
> > + fd = -1;
> > + goto err;
> > + }
> > +
> > + ret = connect(fd, &su, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(su.sun_path+1));
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> > + if (ret < 0) {
>
> This is strange code, connect() returns -1 on error, I have doubts you
> will ever see ret < 0 and why connect() return value is not enough,
> why we need 1 + strlen(su.sun_path+1))?
AFAIK and AFAICS the plymouth socket has a leading NULL byte. That is that PLYMOUTH_SOCKET_PATH
is defined as "\0/org/freedesktop/plymouthd". If I would not add both +1 the NULL byte would
destroy first the result of strlen() as well the NULL byte would not be counted as well.
>
> > + if (errno != ECONNREFUSED)
> > + warnx(_("can not connect on UNIX socket"));
> > + close(fd);
> > + fd = -1;
> > + goto err;
> > + }
> > +err:
> > + return fd;
> > +}
> > +
> > +int plymouth_command(int cmd, ...)
> > +{
> > + char answer[2], command[2];
> > + struct sigaction sp, op;
> > + int fdsock = -1, ret = 0;
> > +
> > + sigemptyset (&sp.sa_mask);
> > + sp.sa_handler = SIG_IGN;
> > + sp.sa_flags = SA_RESTART;
> > + sigaction(SIGPIPE, &sp, &op);
> > +
> > + command[1] = '\0';
> > + switch (cmd) {
> > + case MAGIC_PING:
> > + fdsock = open_un_socket_and_connect();
> > + if (fdsock >= 0) {
> > + command[0] = cmd;
>
> maybe define cmd as char or uint8_t
Yep, but does not hurt I guess.
>
> > + write_all(fdsock, command, strlen(command)+1);
>
> why we need strlen(command)+1 everywhere if the commend is just one
> char+\0, sizeof(command) would be enough.
It is plymouthd which does require the lasting NULL byte here as the
command read by plymouthd has always the size of uint8_t header[2]
compare with ply_boot_connection_read_request() in src/ply-boot-server.c
>
> > + }
> > + break;
> > + case MAGIC_QUIT:
> > + fdsock = open_un_socket_and_connect();
> > + if (fdsock >= 0) {
> > + command[0] = cmd;
> > + write_all(fdsock, command, strlen(command)+1);
> > + }
> > + break;
> > + default:
> > + warnx(_("the plymouth request %c is not implemented"), cmd);
> > + case '?':
> > + goto err;
> > + }
> > +
> > + answer[0] = '\0';
> > + if (fdsock >= 0) {
> > + if (can_read(fdsock, 1000))
> > + read_all(fdsock, &answer[0], sizeof(answer));
> > + close(fdsock);
> > + }
> > + sigaction(SIGPIPE, &op, NULL);
> > + ret = (answer[0] == ANSWER_ACK) ? 1 : 0;
> > +err:
> > + return ret;
> > +}
>
> ...
>
> > /*
> > * Fix the tty modes and set reasonable defaults.
> > */
> > @@ -138,8 +103,9 @@ static void tcinit(struct console *con)
> > struct termios lock;
> > int fd = con->fd;
> > #ifdef TIOCGLCKTRMIOS
> > - int i = (plymouth_command("--ping")) ? 20 : 0;
> > -
> > + int i = (plymouth_command(MAGIC_PING)) ? 20 : 0;
> > + if (i)
> > + plymouth_command(MAGIC_QUIT);
>
> would be possible to #define PLYMOUTH_TERMIOS_FLAGS_DELAY,
> now we have 20s in sulogins and 30s in agetty. Why?
Ahh ... indeed this is a good point :)
Werner
--
"Having a smoking section in a restaurant is like having
a peeing section in a swimming pool." -- Edward Burr
Attachment:
signature.asc
Description: PGP signature
