GNOME Bugzilla – Bug 436808
Signedness of buf in gdm_fdgetc() in daemon/misc.c
Last modified: 2007-05-09 04:26:53 UTC
Hi, While going over the gdm diff, I found a change which doesn't seem to be Debian specific. It changes a "char" buffer to "unsigned char". This sound safer as the signedness of char changes between arches. (Patch follows) Bye,
Created attachment 87781 [details] [review] Declare buf as unsigned
I notice read () calls in all the following places by doing a recursive grep. All of them seem to use just char, and not unsigned char. Why should we change just this one call to read () and not all of them? --- ./gui/gdmuser.c: while (read (STDIN_FILENO, buf, 1) == 1) ./gui/gdmuser.c: size = read (STDIN_FILENO, buf, sizeof (buf)); ./gui/gdmuser.c: while (read (STDIN_FILENO, buf, 1) == 1) ./gui/gdmuser.c: size = read (STDIN_FILENO, buf, sizeof (buf)); ./gui/gdmuser.c: while (read (STDIN_FILENO, buf, 1) == 1) ./gui/gdmuser.c: while ((n = read (STDIN_FILENO, buffer, ./gui/gdmuser.c: read (STDIN_FILENO, buf, sizeof (buf)); ./gui/gdmcomm.c: while (read (fd, buf, 1) == 1 && ./utils/gdmprefetch.c: while (read (fd, buffer, SIZE) != 0) ./daemon/errorgui.c: VE_IGNORE_EINTR (bytes = read (p[0], buf, BUFSIZ-1)); ./daemon/errorgui.c: VE_IGNORE_EINTR (bytes = read (p[0], buf, BUFSIZ-1)); ./daemon/errorgui.c: VE_IGNORE_EINTR (bytes = read (p[0], buf, BUFSIZ-1)); ./daemon/server.c: VE_IGNORE_EINTR (read (server_signal_pipe[0], buf, 4)); ./daemon/cookie.c: VE_IGNORE_EINTR (r = read (fd, buf, MIN (sizeof (buf), rngs[i].length))); ./daemon/slave.c: VE_IGNORE_EINTR (r = read (d->session_output_fd, buf, sizeof (buf))); ./daemon/slave.c: VE_IGNORE_EINTR (read (slave_waitpid_r, buf, 1)); ./daemon/slave.c: VE_IGNORE_EINTR (n = read (fd, buf, sizeof (buf))); ./daemon/slave.c: VE_IGNORE_EINTR (bytes = fread (buf, sizeof (char), ./daemon/slave.c: VE_IGNORE_EINTR (bytes = read (fromfd, buf, sizeof (buf))); ./daemon/slave.c: VE_IGNORE_EINTR (count = read (d->slave_notify_fd, buf, sizeof (buf) -1)); ./daemon/slave.c: VE_IGNORE_EINTR (in_buffer_len = read (pipe1[0], in_buffer, ./daemon/misc.c: VE_IGNORE_EINTR (bytes = read (fd, buf, 1)); ./daemon/misc.c: retval = g_strdup (defread (key)); ./daemon/gdm-net.c: VE_IGNORE_EINTR (len = read (conn->fd, buf, sizeof (buf) -1));
I suppose more of them might need some fixing, yes. Looking harder, I found a reference to the change in the changelog: ... * Ensure char is unsigned in fd_getc, use the utf8 string in the standard greeter (closes: #217496) See Debian bug http://bugs.debian.org/217496. Bye,
Okay, this makes sense to me now. I understand why this particular read() requires unsigned int, since the login code passes the entry data via ve_locale_from_utf8. This is now committed to 2.18 and SVN head.