Hi, On Tue, Apr 22, 2014 at 02:20:21PM +0200, Amadeusz Sławiński wrote: > I also pushed patch for fixing login length limit, because it was also > 20 chars, but can have up to 32.
I think you refer to http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=b62e4ef0971fc9bbf5298d810819f406abb23862 A few remarks to that commit: * With that commit you can also close https://savannah.gnu.org/bugs/?21653 Yay! :-) * 32 seems a little bit short to me. Why not use what the system provides? According to https://bugs.debian.org/560231 at least on Linux there is LOGIN_NAME_MAX defined in /usr/include/bits/local_lim.h which seems to default to 256 nowadays. I think screen should support at least that on Linux then, too. I hence propose to check if LOGIN_NAME_MAX is defined and if so, use that, otherwise use a fixed value, maybe 32 to be on the save side for ancient OS. The patch proposed in https://lists.gnu.org/archive/html/screen-devel/2011-05/msg00000.html (see also below) proposed to use 50, but I prefer numbers which are powers of two. (Debian uses 50 currently, too, though.) If wanted, I can try to figure out a patch for that. * Can we please make that value a single "#define" somewhere for once and for all, so that the next time we have to change that, we only have to change it at one single place? (Unless we missed some code parts. :-) Here's a patch which did that (applied in the Debian package, but won't apply anymore if Amadeusz's patch is already applied): http://anonscm.debian.org/gitweb/?p=collab-maint/screen.git;a=blob;f=debian/patches/49long-usernames.patch (based on https://lists.gnu.org/archive/html/screen-devel/2011-05/msg00000.html, but that one missed at least one more place where that restriction was hardcoded, see https://bugs.debian.org/735554) * You seem to have missed the same multiuser code part as the patch in https://lists.gnu.org/archive/html/screen-devel/2011-05/msg00000.html and as explained in https://bugs.debian.org/735554 -- it's even visible in http://git.savannah.gnu.org/cgit/screen.git/commit/?h=screen-v4&id=b62e4ef0971fc9bbf5298d810819f406abb23862 See this hunk: diff --git a/src/screen.c b/src/screen.c index 6e19732..fd6eb2f 100644 --- a/src/screen.c +++ b/src/screen.c @@ -978,7 +978,7 @@ char **av; if (home == 0 || *home == '\0') home = ppp->pw_dir; - if (strlen(LoginName) > 20) + if (strlen(LoginName) > 32) Panic(0, "LoginName too long - sorry."); #ifdef MULTIUSER if (multi && strlen(multi) > 20) ^^ That last line needs to be changed to 32 (or LOGIN_NAME_MAX or MAX_USERNAME_LEN or whatever), too, because the whole #ifdef reads as follows: #ifdef MULTIUSER if (multi && strlen(multi) > 20) Panic(0, "Screen owner name too long - sorry."); #endif Kind regards, Axel -- /~\ Plain Text Ribbon Campaign | Axel Beckert \ / Say No to HTML in E-Mail and News | a...@deuxchevaux.org (Mail) X See http://www.nonhtmlmail.org/campaign.html | a...@noone.org (Mail+Jabber) / \ I love long mails: http://email.is-not-s.ms/ | http://noone.org/abe/ (Web)
