Skip to content

Conversation

@bagder
Copy link
Member

@bagder bagder commented Dec 13, 2025

libcurl supports up to 8MB string inputs, the config file accepts up to 10MB line lengths. It did not make sense to limit the globs to a maximum of one megabyte.

libcurl supports up to 8MB string inputs, the config file accepts up to
10MB line lengths. It did not make sense to limit the globs to a maximum
of one megabyte.
@bagder bagder marked this pull request as ready for review December 13, 2025 12:51
@bagder bagder requested a review from Copilot December 13, 2025 12:51
@bagder
Copy link
Member Author

bagder commented Dec 13, 2025

augment review

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the maximum URL glob buffer size limit to align with the config file line length limit. The MAX_CONFIG_LINE_LENGTH constant (10MB) is moved from tool_parsecfg.c to the shared header tool_cfgable.h, allowing tool_urlglob.c to use the same limit instead of its previous hardcoded 1MB limit.

Key changes:

  • Moves MAX_CONFIG_LINE_LENGTH definition to a shared header for reusability
  • Updates URL glob buffer initialization to support config line length-sized URLs (10MB instead of 1MB)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/tool_cfgable.h Adds the MAX_CONFIG_LINE_LENGTH constant definition (10MB) for shared use across source files
src/tool_parsecfg.c Removes the local MAX_CONFIG_LINE_LENGTH definition, now using the shared constant from tool_cfgable.h
src/tool_urlglob.c Changes glob buffer initialization from hardcoded 1MB limit to use MAX_CONFIG_LINE_LENGTH (10MB)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder bagder closed this in 28d2757 Dec 13, 2025
@bagder bagder deleted the bagder/large-globs branch December 13, 2025 13:26
@testclutch
Copy link

Analysis of PR #19960 at cab4ee76:

Test 2029 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

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

Development

Successfully merging this pull request may close these issues.

2 participants