Skip to content

Conversation

@bpintea
Copy link
Collaborator

@bpintea bpintea commented Mar 27, 2020

This fixes the debug printing of SQL(U)LEN variable types.
This type is defined as a long on WIN32, but a __int64 on WIN64.
The "%llu"/"%lld" specifier had been used from the start, before the need
for an x86 driver was considered and stayed like that mostly since.

The values logged by an x86 drivers contains the actual values of the
variable, but one needs to convert to hexa, remove the upper 4 byte and
convert back to decimal, which is cumbersome.

The values are now promoted to a (u)int64_t in the logging statements,
so printf will always log the correct value.

This change also updates some other few incorrect specifiers.

This fixes the debug printing of SQL(U)LEN variable types.
This type is defined as a long on WIN32, but a __int64 on WIN64.
The "%llu"/"%lld" specifier had be used from the start, before the need
for an x86 driver was considered and stayed like that mostly since.

The values logged by an x86 drivers contains the actual values of the
variable, but one needs to convert to hexa, remove the upper 4 byte and
convert back, which is cumbersome.

The values are now promoted to a (u)int64_t in the logging statements,
so printf will always log the correct value.

This change also updates some other few incorrect specifiers.
driver/tracing.h Outdated
case 'N': /* long/int64_t unsigned */ \
_n = snprintf(_BUFF + _ps, _AVAIL(_ps), "%llu", \
val ? *(uint64_t *)(uintptr_t)val : 0); \
val ? (int64_t)*(SQLULEN *)(uintptr_t)val : 0); \

Choose a reason for hiding this comment

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

Should this one be uint64_t to match llu?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should indeed, I've updated it. Thanks for catching it.

- fix cast for '%llu' specifier to uint64_t
Copy link

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@bpintea bpintea merged commit bd81338 into elastic:master Mar 27, 2020
@bpintea bpintea deleted the fix/long_long_debug_printing branch March 27, 2020 13:50
bpintea added a commit that referenced this pull request Mar 30, 2020
* Fix debug printing of SQL(U)LEN variables

This fixes the debug printing of SQL(U)LEN variable types.
This type is defined as a long on WIN32, but a __int64 on WIN64.
The "%llu"/"%lld" specifier had be used from the start, before the need
for an x86 driver was considered and stayed like that mostly since.

The values logged by an x86 drivers contains the actual values of the
variable, but one needs to convert to hexa, remove the upper 4 byte and
convert back, which is cumbersome.

The values are now promoted to a (u)int64_t in the logging statements,
so printf will always log the correct value.

This change also updates some other few incorrect specifiers.

* address review comments

- fix cast for '%llu' specifier to uint64_t

(cherry picked from commit bd81338)
bpintea added a commit that referenced this pull request Mar 30, 2020
* Fix debug printing of SQL(U)LEN variables

This fixes the debug printing of SQL(U)LEN variable types.
This type is defined as a long on WIN32, but a __int64 on WIN64.
The "%llu"/"%lld" specifier had be used from the start, before the need
for an x86 driver was considered and stayed like that mostly since.

The values logged by an x86 drivers contains the actual values of the
variable, but one needs to convert to hexa, remove the upper 4 byte and
convert back, which is cumbersome.

The values are now promoted to a (u)int64_t in the logging statements,
so printf will always log the correct value.

This change also updates some other few incorrect specifiers.

* address review comments

- fix cast for '%llu' specifier to uint64_t

(cherry picked from commit bd81338)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants