Skip to content

ODBC Mars issues #690

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

Open
mmcnabb-vms wants to merge 10 commits into
base: master
Choose a base branch
from
+139 −25
Open

ODBC Mars issues #690

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e782a89
ODBC MARS: Fix sliding window calculation
mmcnabb-vms Dec 23, 2025
675c58e
ODBC MARS: Extra debug logging
mmcnabb-vms Dec 23, 2025
138584b
ODBC MARS: Don't need to free MARS socket until statement free
mmcnabb-vms Dec 24, 2025
3ba42b5
ODBC Tests: option to suppress display of every command
mmcnabb-vms Dec 24, 2025
c309dfc
ODBC MARS: Expand the MARS1 test
mmcnabb-vms Dec 24, 2025
148c56b
ODBC MARS: Fix sliding window calculation (updated)
mmcnabb-vms Dec 29, 2025
533f6b4
ODBC MARS: Don't need to free MARS socket until statement free (updated)
mmcnabb-vms Dec 29, 2025
149edc9
Remove accidental debugging
mmcnabb-vms Dec 30, 2025
8f8e509
ODBC MARS test: Heap usage logging
mmcnabb-vms Dec 29, 2025
a18a448
Move declarations to top of function , for old compiler appveyor
mmcnabb-vms Jan 16, 2026
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
8 changes: 5 additions & 3 deletions src/tds/packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ int
tds_read_packet(TDSSOCKET * tds)
{
#if ENABLE_ODBC_MARS
TDS_UINT this_seq;
TDSCONNECTION *conn = tds->conn;

tds_mutex_lock(&conn->list_mtx);
Expand Down Expand Up @@ -554,9 +555,10 @@ tds_read_packet(TDSSOCKET * tds)
tds->in_pos = 8;
tds->in_flag = tds->in_buf[0];

/* send acknowledge if needed */
if ((int32_t) (tds->recv_seq + 2 - tds->recv_wnd) >= 0)
tds_update_recv_wnd(tds, tds->recv_seq + 4);
/* Look ahead by up to 4 packets */
memcpy(&this_seq, ((char const *)packet->buf) + offsetof(TDS72_SMP_HEADER, seq), sizeof this_seq);
Copy link
Contributor

@freddy77 freddy77 Dec 24, 2025

Choose a reason for hiding this comment

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

I would use

this_seq = TDS_GET_A4LE(&((const TDS72_SMP_HEADER *) packet->buf)->seq);

also, consider that other code will still use tds->recv_seq, perhaps it would be better to update that in a different way.

Copy link
Contributor Author

@mmcnabb-vms mmcnabb-vms Dec 29, 2025

Choose a reason for hiding this comment

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

Agree about using TDS_GET_A4LE ; but is packet guaranteed to be correctly aligned for that cast?

The code in tds_connection_put_packet makes the same cast, so I guess it's not a problem. The code in tds_packet_read() uses memcpy(&mars_header, packet->buf, sizeof(mars_header));.

Copy link
Contributor

@freddy77 freddy77 Jan 24, 2026

Choose a reason for hiding this comment

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

I though I replied to this. Packet is an allocated buffer so it's surely aligned to 32 bit.

if (this_seq + 2 >= tds->recv_wnd)
Copy link
Contributor

@freddy77 freddy77 Dec 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This formulae is wrong if you have wrapping, the old one was correct.

Copy link
Contributor Author

@mmcnabb-vms mmcnabb-vms Dec 29, 2025

Choose a reason for hiding this comment

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

OK, you're right about the wrapping issue. I didn't like the old one as the C Standard doesn't define the behaviour of converting out-of-range unsigned values to signed integer, but FreeTDS is unlikely to be used on any system that doesn't behave as expected here.

Perhaps we could improve readability by borrowing TCP sequence macros, e.g. from BSD but with TDS_INT as the cast type (it should be the signed counterpart of TDS_UINT).

tds_update_recv_wnd(tds, this_seq + 4);

return tds->in_len;
}
Expand Down