Skip to content

ICU-22813 Rise the size of the buffers used for the command strings at pkgdata#3058

Merged
markusicu merged 1 commit into
unicode-org:mainfrom
clopez:pkgdata-risebufsize
Jul 19, 2024
Merged

ICU-22813 Rise the size of the buffers used for the command strings at pkgdata#3058
markusicu merged 1 commit into
unicode-org:mainfrom
clopez:pkgdata-risebufsize

Conversation

@clopez

@clopez clopez commented Jul 1, 2024

Copy link
Copy Markdown
Contributor

The tool pkgdata uses snprintf() to build the strings of the commands that will execute later during the install process. But the maximum size of this buffers is not enough when there is a long path.

This has caused issues on some CI systems that use very long paths, causing the install process to produce a wrong result.

The maximum path on Linux is 4096 (defined as PATH_MAX at <linux/limits.h>) So the size of SMALL_BUFFER_MAX_SIZE should be 4096 to avoid errors related to truncated paths.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22813
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

…t pkgdata

The tool pkgdata uses snprintf() to build the strings of the commands that
will execute later during the install process. But the maximum size of this
buffers is not enough when there is a long path.

This has caused issues on some CI systems that use very long paths, causing
the install process to produce a wrong result.

The maximum path on Linux is 4096 (defined as PATH_MAX at <linux/limits.h>)
So the size of SMALL_BUFFER_MAX_SIZE should be 4096 to avoid errors related
to truncated paths.
@CLAassistant

CLAassistant commented Jul 1, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@markusicu markusicu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm tnx!
Longer-term, we should rewrite this kind of code to print into a std::string, which we can use here since it's not part of the ICU runtime libraries.

@markusicu markusicu self-assigned this Jul 18, 2024
@markusicu markusicu merged commit 8ca6bc7 into unicode-org:main Jul 19, 2024
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

Successfully merging this pull request may close these issues.

3 participants