Brian Cameron wrote: > > Robert: > > There is something wrong with the updated patch. When I try to > run "gpatch -p0 < solaris-gdm.patch" it complains with this error: > > patch: **** unexpected end of file in patch > > Could you fix that? >
Oops sorry 'bout that, fixed now. >>> I was hoping you could also file a bug at bugzilla.gnome.org in the >>> "gdm" category. Since this is a general GDM issue, not a Sun specific >>> one. >>> >> While I agree that the patch makes the code "more correct" in the >> general sense, the problem only occurs on Solaris not on Linux. The >> glibc versions of getnameinfo(), getaddrinfo() and sendto (called by >> XdmcpFlush) only check that the sockaddr is >= to the expected >> length. Solaris on the other hand checks to see that the length is >> == to the expected length. >> >> So in a sense this is a Solaris specific problem. > > I understand, but I think it is still appropriate to go upstream. It > shouldn't break anything on Linux. As you say, it is also "more > correct" to pass the right length. Just because some distros don't > check isn't a good reason to pass the wrong length. > I wasn't saying that it shouldn't go upstream. I meant that, since it doesn't happen on other platforms, you would have more luck getting it integrated as a Solaris compatibility change than I would as an general bug fix. >> The calls to XdmcpFlush fail so no packets are sent (locally or >> remotely). This means that xdmcp doesn't work at all. The host >> doesn't appear in any chooser, remote or local. > > Thanks for explaining more clearly. This will allow me to explain the > reason for the change in the GDM ChangeLog file more clearly. > >>> I'm still a bit uncomfortable with this change. Looking at how >>> gdm_address_peek_local_list is used, I don't see it is only used for >>> logging purposes. I see it is used in the following ways: >>> >> You are confusing the change to misc.c with the one to gdm-common.c. >> The removal of NI_NUMERICHOST and NI_NUMERICSERV is in the routine >> gdm_address_get_info() not gdm_address_peek_local_list(). > > Whoops, thanks - yes you are right. > >> However, in reviewing all the calls to gdm_address_get_info() there >> are 2 places out of 31 where the numeric version is actually wanted. >> I updated the patch on the ftp site to remove that change. > > Perhaps it might make sense to make two functions, or make the function > pass in a boolean so you can request which to return? > >> I originally made the change to simplify debugging the xdmcp >> problem. Since it is primarily cosmetic, I'll redo the change and >> fix the other locations that really did want the numeric address and >> submit it directly to gnome.org. > > Yes, let's leave this out of the fix for now. If you want to improve > this logic further, please provide a patch to bugzilla.gnome.org and > I'll apply that separately. > >> Thanks for the feedback. > > My pleasure. Thanks for helping make GDM work better on Solaris. > >> Your feedback has been helpful and not the least bit pedantic. > > Thanks, > > Brian
