[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[ debian-hurd-Patches-301172 ] #218624: krb4



Patches item #301172, was opened at 2005-02-11 17:16
You can respond by visiting: 
http://alioth.debian.org/tracker/?func=detail&atid=410472&aid=301172&group_id=30628

Category: linuxism
>Group: submitted
Status: Open
Resolution: None
Priority: 9
Submitted By: Michael Banck (mbanck)
Assigned to: Nobody (None)
Summary: #218624: krb4

Initial Comment:
Author: Guillem Jover <guillem@debian.org>
Source: krb4
Status: Not applied (#218624)
Maturity: Needs revising (NCARGS)
Strip-Level: debian/patches/036_hurd
Hurd-Specific: yes


----------------------------------------------------------------------

Comment By: Marcus Brinkmann (brinkmd)
Date: 2005-02-18 18:57

Message:
Logged In: YES 
user_id=1127

Hi,

just a few comments on the patch (and I also looked at the bug report):

Using MaxHostNameLen may be fine.  I have no idea why it is there, but it is, and it is used widely in the package, so maybe the remaining occurences of MAXHOSTNAMELEN have just been slipped through the conversion.

Using NCARGS for statically allocated memory is _WRONG_.  It will blow up in your face.  For example, the current code may trash the last machine word on the stack, depending on the compiler, or do other horrible things.  Ok, I give it to you: It's not nice of the Hurd to define NCARGS to INT_MAX ;)  But the bsd_locl.h header already defines NCARGS to 1MB if it is not defined.  So, two options (both without changing the assumption in the code that there should be a limit in the first place):

Use ARG_MAX, which is POSIX.  If ARG_MAX is not defined, use 1MB.  This will work on the Hurd, although I find the thought of allocating 1MB on the stack appealing, or fallback to the second option (in case ARG_MAX is not defined but NCARGS is):

If NCARGS is defined, but larger than 1MB, use 1MB.

The ioctl is pretty bogus.  We do not implement an AFS filesystem, so it is useless at least.  And using internal macro definitions is just not kosher.  Instead, please just use --disable-afs-support and make it work.  For example, there is an obvious typo in configure.in that checks $enabls_afs_support, so currently even if you use the configure option, NO_AFS is not defined.  --disable-afs-support should just be made to work and then the code with the ioctl should not be compiled (I say should, I did not verify this).


----------------------------------------------------------------------

Comment By: Michael Banck (mbanck)
Date: 2005-02-11 17:51

Message:
Logged In: YES 
user_id=2380

Author: Guillem Jover <guillem@debian.org>
Source: krb4
Status: submitted(#218624), needs-work (NCARGS)
Categories: posix, linuxism
Strip-Level: debian/patches/036_hurd

----------------------------------------------------------------------

You can respond by visiting: 
http://alioth.debian.org/tracker/?func=detail&atid=410472&aid=301172&group_id=30628



Reply to: