Skip to content

Maintenance: Removed deprecated Client::replyBodySpace() #2060

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.

Sign up for GitHub

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

Closed
eduard-bagdasaryan wants to merge 2 commits into from
Closed

Maintenance: Removed deprecated Client::replyBodySpace() #2060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff6a778
Removed deprecated Client::replyBodySpace()
eduard-bagdasaryan May 8, 2025
5efe76c
Merge branch 'master' into SQUID-1075-remove-duplicated-deprecated-re…
eduard-bagdasaryan May 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions src/clients/Client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,44 +1110,3 @@ Client::calcBufferSpaceToReserve(size_t space, const size_t wantSpace) const
return space;
}

size_t
Client::replyBodySpace(const MemBuf &readBuf, const size_t minSpace) const
{
size_t space = readBuf.spaceSize(); // available space w/o heroic measures
if (space < minSpace) {
const size_t maxSpace = readBuf.potentialSpaceSize(); // absolute best
space = min(minSpace, maxSpace); // do not promise more than asked
}

#if USE_ADAPTATION
if (responseBodyBuffer) {
return 0; // Stop reading if already overflowed waiting for ICAP to catch up
}

if (virginBodyDestination != nullptr) {
/*
* BodyPipe buffer has a finite size limit. We
* should not read more data from the network than will fit
* into the pipe buffer or we _lose_ what did not fit if
* the response ends sooner that BodyPipe frees up space:
* There is no code to keep pumping data into the pipe once
* response ends and serverComplete() is called.
*
* If the pipe is totally full, don't register the read handler.
* The BodyPipe will call our noteMoreBodySpaceAvailable() method
* when it has free space again.
*/
size_t adaptation_space =
virginBodyDestination->buf().potentialSpaceSize();

debugs(11,9, "Client may read up to min(" <<
adaptation_space << ", " << space << ") bytes");

if (adaptation_space < space)
space = adaptation_space;
}
#endif

return space;
}

2 changes: 0 additions & 2 deletions src/clients/Client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,6 @@ class Client:
void adaptOrFinalizeReply();
void addVirginReplyBody(const char *buf, ssize_t len);
void storeReplyBody(const char *buf, ssize_t len);
/// \deprecated use SBuf I/O API and calcBufferSpaceToReserve() instead
size_t replyBodySpace(const MemBuf &readBuf, const size_t minSpace) const;
/// determine how much space the buffer needs to reserve
size_t calcBufferSpaceToReserve(const size_t space, const size_t wantSpace) const;

Expand Down
4 changes: 3 additions & 1 deletion src/clients/FtpClient.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,9 @@ Ftp::Client::maybeReadVirginBody()

initReadBuf();

const int read_sz = replyBodySpace(*data.readBuf, 0);
// XXX: We only use this call to decide whether to read; we never increase data.readBuf space.
// TODO: Upgrade data.readBuf to SBuf and merge this with similar HttpStateData::readReply() code.
const auto read_sz = calcBufferSpaceToReserve(data.readBuf->spaceSize(), data.readBuf->spaceSize());

@rousskov rousskov May 8, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • The second parameter in removed replyBodySpace() is called minSpace. We passed 0.
  • The second parameter in calcBufferSpaceToReserve() is called wantSpace. We now pass spaceSize().

In both cases, the end result is exactly the same!

The value change makes sense because parameter names differ, and buffer spaceSize() is much closer to wantSpace than 0 would be -- we do not want to get rid of the existing buffer space. Neither value is actually "ideal" or "best", but improving requires difficult refactoring that lies outside this no-behavior-changes cleanup PR. We have several related TODOs/XXXs and runtime bugs. This small/simple PR helps with ongoing refactoring that will eventually address those. It is a small but useful step forward.

This comment does not request any PR changes.


debugs(9, 9, "FTP may read up to " << read_sz << " bytes");

Expand Down