[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [screen-devel] Remarks on the patch for long login names (was: Re: G
From: |
Amadeusz Sławiński |
Subject: |
Re: [screen-devel] Remarks on the patch for long login names (was: Re: GNU Screen v.4.2.0) |
Date: |
Tue, 22 Apr 2014 18:32:07 +0200 |
On Tue, 22 Apr 2014 17:39:17 +0200
Axel Beckert <address@hidden> wrote:
> 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! :-)
Yes, I will look through bugzilla
I'm pretty sure this one is also fixed with altscreen patch for example:
https://savannah.gnu.org/bugs/?35757
> * 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.
>
Ah, I based my value on utmp entries
bits/utmp.h:#define UT_NAMESIZE 32
bits/utmp.h: char ut_user[UT_NAMESIZE]; /* Username. */
bits/utmpx.h:#define __UT_NAMESIZE 32
bits/utmpx.h: char ut_user[__UT_NAMESIZE]; /* Username. */
Well, let's just use 256, it will probably allow for use with most
crazy login schemes.
> 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.)
In previous mail Jürgen suggested that it's bad idea to have build time
changing defines in struct msg. I will hardcode them.
> 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. :-)
>
Found a perfect place, os.h at bottom allows for user defined values.
> 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
>
Yes, thanks for checking.
Amadeusz
- Re: [screen-devel] GNU Screen v.4.2.0, (continued)
- Re: [screen-devel] GNU Screen v.4.2.0, Jeroen Roovers, 2014/04/18
- Re: [screen-devel] GNU Screen v.4.2.0, Jim Mahood, 2014/04/18
- Re: [screen-devel] GNU Screen v.4.2.0, Amadeusz Sławiński, 2014/04/18
- Re: [screen-devel] GNU Screen v.4.2.0, Jürgen Weigert, 2014/04/18
- Re: [screen-devel] GNU Screen v.4.2.0, Amadeusz Sławiński, 2014/04/22
- [screen-devel] Remarks on the patch for long login names (was: Re: GNU Screen v.4.2.0), Axel Beckert, 2014/04/22
- Re: [screen-devel] Remarks on the patch for long login names (was: Re: GNU Screen v.4.2.0),
Amadeusz Sławiński <=
- Re: [screen-devel] Remarks on the patch for long login names (was: Re: GNU Screen v.4.2.0), Axel Beckert, 2014/04/22
Re: [screen-devel] GNU Screen v.4.2.0, Jostein Berntsen, 2014/04/24
- Re: [screen-devel] GNU Screen v.4.2.0, Jürgen Weigert, 2014/04/25
- Message not available
- Re: [screen-devel] GNU Screen v.4.2.0, Jostein Berntsen, 2014/04/25
- Re: [screen-devel] GNU Screen v.4.2.0, Jürgen Weigert, 2014/04/25
- Message not available
- Message not available
- Re: [screen-devel] GNU Screen v.4.2.0, Jostein Berntsen, 2014/04/25
- Re: [screen-devel] GNU Screen v.4.2.0, Jürgen Weigert, 2014/04/25
- Message not available
- Re: [screen-devel] GNU Screen v.4.2.0, Jostein Berntsen, 2014/04/25
- Re: [screen-devel] GNU Screen v.4.2.0, Jürgen Weigert, 2014/04/25
- Re: [screen-devel] GNU Screen v.4.2.0, Amadeusz Sławiński, 2014/04/25