Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mem waste / Cleanup buffer size and functions for "nick!userhost" glueing #650

Closed
michaelortmann opened this issue Sep 26, 2018 · 3 comments

Comments

@michaelortmann
Copy link
Member

michaelortmann commented Sep 26, 2018

There are many places within eggdrop, where "nick!userhost" is glued togather like this:

egg_snprintf(s, sizeof s, "%s!%s", m->nick, m->userhost);

Often the used function differs. Sometimes sprintf, sometimes, egg_snprintf, maybe others as well.

Often the used buffer sitze differ. Sometimes static 512, sometimes dynamic, maybe others as well.

It should always be exactly NICKLEN + UHOSTLEN.

And we should always use the same function, be it egg_snprintf or sprintf.

By fixing the buffer size, we fix truncation or regain precious memory, depends if the buffer was too small or too big

For 2 such cases it will be done by #649.

@Cizzle
Copy link
Member

Cizzle commented Oct 27, 2018

As mentioned in another issue/pr; from eggdrop.h:

#define HANDLEN    32 /* valid values 9->NICKMAX                             */
#define NICKMAX    32 /* valid values HANDLEN->32                            */
#define UHOSTMAX   291 + NICKMAX /* 32 (ident) + 3 (\0, !, @) + NICKMAX */
#define NICKLEN    NICKMAX + 1
#define UHOSTLEN   UHOSTMAX + 1

Meaning NICKLEN + UHOSTLEN doesn't give what you think you'll get.

@michaelortmann
Copy link
Member Author

michaelortmann commented Oct 27, 2018

Yes, thats why we need to fix the definitions in eggdrop.h, starting with anylyzing what UHOSTLEN should be instead of what it is now. where does the 291 even come from? isn't the whole nick+userhost == nick!user@host limited to the sum of its lenghts, so we need to start with the parts of this nickuserhost... like... hostname should be 63 chars max (rfc). nick is well.. the max length of the nickname we already have as an eggdrop setting (005 later) but ok. 32 is also a sane upper bound we dont have to care anymore, maybe that 32 fuer nickmax/handlen is the only value in eggdrop.h in this block, that actually makes sense. and user certainly has a limit as well, that we should code into eggdrop.h and all others can be sums of those...

UHOSTMAX is dual used now.
once it is used as length for user@host
and then it is also used for nick!user@host cases.
that is the problem rephrased, that needs to be solved here.

The use of UHOSTMAX and UHOSTLEN within eggdrop isnt clear
so we must go over all those places and check what is meant:
max or len
user@host or nick!user@host.

And propose to start from something like this:

#define NICKMAX 32 /* unchanged */
#define USERMAX 32 /* ident and/or ircd usually caps at 10 or 32, though i dont think its an rfc limit */
#define HOSTMAX 255 */ irc rfc says 63, but a misconfigured ircd can support FQDN */
#define UHOSTMAX USERMAX + 1 + HOSTMAX /* user@host*/
#define NUHOSTMAX NICKMAX + 1 + UHOSTMAX /* nick!user@host */

Is NUHOSTMAX a good name?
All the *LEN defines are *MAX +1 for the temrinating \0

@michaelortmann
Copy link
Member Author

Meanwhile, this has been fixed by #1585

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants