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

Add constants for max IP and SteamID length #566

Merged
merged 5 commits into from
Sep 28, 2018

Conversation

OciXCrom
Copy link
Contributor

I'm just not sure if the max SteamID length is 34. I wasn't able to find the best answer.

@WPMGPRoSToTeMa
Copy link
Contributor

It should be _SIZE, because _LENGTH is _SIZE - 1 for C strings, but anyway MAX_NAME_LENGTH is already here.

@OciXCrom
Copy link
Contributor Author

@WPMGPRoSToTeMa I know, but I made it _LENGTH for consistency with MAX_NAME_LENGTH.

@Arkshine
Copy link
Member

Looks good for me. Do you see something else, @WPMGPRoSToTeMa?

@WPMGPRoSToTeMa
Copy link
Contributor

WPMGPRoSToTeMa commented Sep 11, 2018

@Arkshine constants for resource path, userinfo, motd, menu length?

@Arkshine
Copy link
Member

Resource path -> 64
Motd -> don't remember, ~1280?

What do you mean by 'userinfo', and 'menu length'?

@WPMGPRoSToTeMa
Copy link
Contributor

WPMGPRoSToTeMa commented Sep 12, 2018

@Arkshine motd -> 1536.

What do you mean by 'userinfo'

get_user_info, copy_infokey_buffer, etc. natives, setinfo command -> 256.

and 'menu length'?

Menu contents length -> 512.

@Arkshine
Copy link
Member

Sounds good.

@SergeyShorokhov
Copy link
Contributor

Don't forget PLATFORM_MAX_PATH

@Arkshine
Copy link
Member

Arkshine commented Sep 12, 2018

It's defined to 260 because this would be the max path length in windows.
But the resource files you precache through the engine and sent to the client is limited to 64 characters.

@SergeyShorokhov
Copy link
Contributor

SergeyShorokhov commented Sep 12, 2018

also don't forget #define MAX_QUERY_LENGTH 1472. Why 1472? - idk

@OciXCrom
Copy link
Contributor Author

I added these:

#define MAX_RESOURCE_PATH_LENGTH 64
#define MAX_MOTD_LENGTH 1536
#define MAX_USER_INFO_LENGTH 256
#define MAX_MENU_LENGTH 512
#define MAX_QUERY_LENGTH 1472

@Arkshine
Copy link
Member

also don't forget #define MAX_QUERY_LENGTH 1472. Why 1472? - idk

What kind of query you're talking about? Where did you see this constant?

@WPMGPRoSToTeMa
Copy link
Contributor

@Arkshine I think he's talking about SQL queries.

@Arkshine
Copy link
Member

Why 1472? The limit would be the limit defined by some MySQL options, like maybe max_allowed_packet. But since we are talking about AMXX, it comes down to the AMXX buffer and even before that the compiler. The compiler input line max length is 4095, and it seems MySQL module uses internally a buffer of 4096 to save the query. If anything it should be 4096, but I don't know if it makes sense to define such a large max length.

@rsKliPPy
Copy link
Contributor

Nah, at most the MySQL module should log a warning that the query string has exceeded maximum size. A constant is unnecessary.

@IgnacioFDM
Copy link
Contributor

Compiler input line length is pretty unrelated to query length, since usually (long queries) are built (as in, not just a string literal, but lots of stuff inserted into the string).

I think the 4096 current limit is pretty low btw, it's common to have single INSERT queries that you build with multiple values, instead of thousands of single value queries.

These long INSERT queries are many kilobytes or megabytes long if you don't split them so as to fit with the current limit.

You probably can't, or shouldn't at least, allocate megabytes in the stack of an AMXX plugin , but 16K is probably within reason.

I'm currently using this constant, so it might be useful to add. However I think the name should be different, like SQL_MAX_QUERY_LENGTH and be included in sqlx.inc

@rsKliPPy
Copy link
Contributor

rsKliPPy commented Sep 13, 2018

The solution is to introduce transactions for such queries if they are the main problem.

@OciXCrom
Copy link
Contributor Author

What about the maximum hostname length? Is it 64?

@Arkshine
Copy link
Member

Let's go with that for now. If we need others constants, feel free to open a new PR.

@Arkshine Arkshine merged commit b4768a3 into alliedmodders:master Sep 28, 2018
Arkshine pushed a commit that referenced this pull request Sep 28, 2018
* Add constants for max IP and SteamID length

* Change max authid length to 64

* Fix port typo

* Add more defines

* Remove max query length
@OciXCrom OciXCrom deleted the ipauth-const branch September 28, 2018 17:06
@WPMGPRoSToTeMa
Copy link
Contributor

WPMGPRoSToTeMa commented Sep 28, 2018

/**
* The maximum buffer size that can be displayed in a MOTD.
*/
#define MAX_MOTD_LENGTH 1536
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we don't know current code base.
hl now support color menus. what else has changed in current builds?

Copy link
Contributor

@WPMGPRoSToTeMa WPMGPRoSToTeMa Sep 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mistrick that repository contains colored menu support.

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

Successfully merging this pull request may close these issues.

8 participants