Skip to content

Remove several string copies and unnecessary heap allocations #3222

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

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Remove several string copies and unnecessary heap allocations #3222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bb07de9
toupper/tolower is already receiving a copy, so it doesn't need to cr…
eduar-hte Apr 29, 2024
f8dd09f
Avoid creating a new std::string on the heap to create VariableValue
eduar-hte May 18, 2024
1534ee2
Removed unnecessary copies
eduar-hte May 30, 2024
8b17f36
Inline string functions
eduar-hte May 25, 2024
cc0f893
Removed unused overload of dash_if_empty that sonarcloud flags as pot…
eduar-hte Aug 9, 2024
77adb57
Avoid std::string copy in ssplit argument
eduar-hte Aug 12, 2024
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
1 change: 0 additions & 1 deletion src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ UTILS = \
utils/random.cc \
utils/regex.cc \
utils/sha1.cc \
utils/string.cc \
utils/system.cc \
utils/shared_files.cc

Expand Down
35 changes: 10 additions & 25 deletions src/request_body_processor/multipart.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,20 @@ void MultipartPartTmpFile::Open() {

localtime_r(&tt, &timeinfo);

char tstr[300] {};
strftime(tstr, 299, "/%Y%m%d-%H%M%S", &timeinfo);
char tstr[17];
strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M%S", &timeinfo);

std::string path = m_transaction->m_rules->m_uploadDirectory.m_value;
path = path + tstr + "-" + *m_transaction->m_id.get();
path += "-file-XXXXXX";

char* tmp = strdup(path.c_str());
#ifndef WIN32
m_tmp_file_fd = mkstemp(tmp);
m_tmp_file_fd = mkstemp(path.data());
#else
_mktemp_s(tmp, path.length()+1);
m_tmp_file_fd = _open(tmp, _O_CREAT | _O_EXCL | _O_RDWR);
_mktemp_s(path.data(), path.length()+1);
m_tmp_file_fd = _open(path.c_str(), _O_CREAT | _O_EXCL | _O_RDWR);
#endif
m_tmp_file_name.assign(tmp);
free(tmp);
m_tmp_file_name = path;
ms_dbg_a(m_transaction, 4, "MultipartPartTmpFile: Create filename= " + m_tmp_file_name);

int mode = m_transaction->m_rules->m_uploadFileMode.m_value;
Expand Down Expand Up @@ -857,7 +855,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
}

new_value = std::string(data);
utils::string::chomp(&new_value);
utils::string::chomp(new_value);

/* update the header value in the table */
header_value = m_mpp->m_headers.at(
Expand Down Expand Up @@ -926,7 +924,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
i++;
}
header_value = std::string(data);
utils::string::chomp(&header_value);
utils::string::chomp(header_value);

/* error if the name already exists */
if (m_mpp->m_headers.count(header_name) > 0) {
Expand Down Expand Up @@ -1271,22 +1269,10 @@ int Multipart::multipart_complete(std::string *error) {


int Multipart::count_boundary_params(const std::string& str_header_value) {
std::string lower = utils::string::tolower(str_header_value);
const char *header_value = lower.c_str();
char *duplicate = NULL;
char *s = NULL;
int count = 0;

if (header_value == NULL) {
return -1;
}

duplicate = strdup(header_value);
if (duplicate == NULL) {
return -1;
}

s = duplicate;
const auto lower = utils::string::tolower(str_header_value);
const char *s = lower.c_str();
while ((s = strstr(s, "boundary")) != NULL) {
s += 8;

Expand All @@ -1295,7 +1281,6 @@ int Multipart::count_boundary_params(const std::string& str_header_value) {
}
}

free(duplicate);
return count;
}

Expand Down
4 changes: 2 additions & 2 deletions src/transaction.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
ss << utils::string::dash_if_empty(
m_variableRequestHeaders.resolveFirst("Host").get())
<< " ";
ss << utils::string::dash_if_empty(this->m_clientIpAddress->c_str()) << " ";
ss << utils::string::dash_if_empty(this->m_clientIpAddress.get()) << " ";
/** TODO: Check variable */
variables::RemoteUser *r = new variables::RemoteUser("REMOTE_USER");
std::vector<const VariableValue *> l;
Expand All @@ -1531,7 +1531,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
delete r;

ss << utils::string::dash_if_empty(
m_variableRemoteUser.c_str());
&m_variableRemoteUser);
ss << " ";
/** TODO: Check variable */
//ss << utils::string::dash_if_empty(
Expand Down
268 changes: 0 additions & 268 deletions src/utils/string.cc
View file
Open in desktop

This file was deleted.

Loading
Loading