Skip to content

ftplistparser: no more overwriting set.write_func - #22969

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/ftp-wc-write-func
Closed

bagder wants to merge 1 commit into
masterfrom
bagder/ftp-wc-write-func

Conversation

@bagder

@bagder bagder commented Sep 16, 2026

Copy link
Copy Markdown
Member

The entries in the 'set' struct should remain as set by the application.

@bagder bagder added the FTP label Sep 16, 2026
@bagder
bagder requested a balanced review from Copilot September 16, 2026 13:41
@bagder
bagder marked this pull request as ready for review September 16, 2026 13:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The callback-preservation behavior needs targeted regression coverage, and the new API comment is inaccurate.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Moves FTP wildcard LIST parsing into the output dispatch layer, preserving application-configured write settings.

Changes:

  • Dynamically selects the FTP list parser during wildcard listing.
  • Removes temporary callback backups and restoration.
  • Adds wildcard listing-mode detection.
File summaries
File Description
lib/ftplistparser.h Declares listing-mode detection.
lib/ftplistparser.c Implements listing-mode detection.
lib/ftp.c Stops replacing application write settings.
lib/ftp-int.h Removes obsolete backup storage.
lib/cw-out.c Routes listing output to the parser dynamically.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/cw-out.c
Comment thread lib/ftplistparser.h Outdated
The entries in the 'set' struct should remain as set by the application.

Closes #22969
@bagder
bagder force-pushed the bagder/ftp-wc-write-func branch from 124039e to 335da96 Compare September 16, 2026 13:46
@icing

icing commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

So, let me understand what this does.

The application turns on CURLOPT_WILDCARDMATCH and installs callbacks for CURLOPT_CHUNK_BGN_FUNCTION and CURLOPT_CHUNK_END_FUNCTION. Then curl sends a "LIST" command to the server. The response to "LIST" is intercepted by the wildcard parser that matches entries and collects them all.

Once "LIST" is done, , the matching filenames are passed to CURLOPT_CHUNK_BGN_FUNCTION and, depending on its return code, the file is skipped or downloaded and written out through the usual writer callbacks with calling CURLOPT_CHUNK_END_FUNCTION at the end of a download.

Basically, except it's bit more complicated than that since this all happens inside one easy handle who needs to dance its states around...interesting.😰

Hmm...I think FTP should add a new Curl_writer that catches and parses the response from "LIST". Once it is done, it is removed again and the collected list is acted upon. Since the writer is local to the easy handle, any failure will clean it up. And it is not dup'ed either.

More thinking tomorrow.

@bagder

bagder commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

Basically: yes. And this particular fix is just to stop storing private things in the set.write_func pointer as it meant to only hold the callback set by the application so that duplicate etc always gets the right one and there's no risk that the wrong pointer remains set.

@bagder bagder closed this in ba8f8a7 Sep 17, 2026
@bagder
bagder deleted the bagder/ftp-wc-write-func branch September 17, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants