Skip to content
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.

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

ct.c tuneups from NCBI as of May 2024 #561

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ucko
Copy link
Contributor

@ucko ucko commented May 30, 2024

This PR contains two tuneups to ct.c from #555 that git-deps flagged as interdependent, though it looks like that may have been a false positive.

ucko added 2 commits May 29, 2024 13:07
Explicitly build cmd->iodesc->name and update ...->namelen along the
way rather than calling sprintf and strlen (which can be slow).

Signed-off-by: Aaron M. Ucko <[email protected]>
Report (anticipated) truncation when given data for a variable-width
type whose maximum width is less than the supplied data length.  (The
actual truncation will occur in libtds code that cannot readily
complain about it.)  To that end, copy error code 36's description
from cs.c.

Signed-off-by: Aaron M. Ucko <[email protected]>
@freddy77
Copy link
Contributor

Merging first commit.

About the second, I'm doing some tests. Looks like the check on Sybase library is done while calling ct_param, not sending. Also the error is pretty different, I'm getting (CS_CHAR_TYPE):

Open Client Message: 0x2f658890 0x2f674180
number 0x101018a layer 1 origin 1 severity 1 number 138
msgstring: ct_param(): user api layer: external error: A data length of 25600 exceeds the maximum length allowed for CHAR data.
osstring: (null)
ct_param(char) failed

Also it looks like data for parameters are copied during ct_param (changing them after do not affect the data used in the query).

@ucko
Copy link
Contributor Author

ucko commented Mar 25, 2025

Thanks! I'll take a closer look at the second.

@ucko
Copy link
Contributor Author

ucko commented Mar 25, 2025

Ah, yeah, looks like I went with an existing error code rather than checking what the SAP implementation does.

@freddy77
Copy link
Contributor

No rush. But the difference about ct_param getting the data is more concerning to me (well, not that affect this PR).
Consider a program that:

  • computes value allocating memory;
  • pass value to ct_param;
  • free the value;
  • calls ct_send.
    At the time of ct_send if we attempt to get the data having just saved the pointer value could be garbage or released to OS, so potentially causing corruption (invalid data inserted) or crashes.

Anyway, OT.

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.

2 participants