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?

>> 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.

> 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

Reply via email to