wankdanker / node-odbc

ODBC bindings for node
MIT License
174 stars 79 forks source link

src: fix compilation error on 32-bit linux #25

Closed santigimeno closed 8 years ago

santigimeno commented 9 years ago
santigimeno commented 9 years ago

Without this change i was getting this error:

CXX(target) Release/obj.target/odbc_bindings/src/odbc.o
../src/odbc.cpp: In static member function ‘static v8::Local<v8::Object> ODBC::GetSQLError(SQLSMALLINT, SQLHANDLE, char*)’:
../src/odbc.cpp:887:29: error: call of overloaded ‘New(SQLINTEGER&)’ is ambiguous
       errors->Set(Nan::New(i), subError);
                             ^
../src/odbc.cpp:887:29: note: candidates are:
In file included from ../node_modules/nan/nan.h:200:0,
                 from ../src/odbc.h:23,
                 from ../src/odbc.cpp:25:
../node_modules/nan/nan_new.h:270:1: note: Nan::imp::FactoryBase<v8::Boolean>::return_t Nan::New(bool)
 New(bool value) {
 ^
../node_modules/nan/nan_new.h:276:1: note: Nan::imp::IntegerFactory<v8::Int32>::return_t Nan::New(int32_t)
 New(int32_t value) {
 ^
../node_modules/nan/nan_new.h:282:1: note: Nan::imp::FactoryBase<v8::Uint32>::return_t Nan::New(uint32_t)
 New(uint32_t value) {
 ^
../node_modules/nan/nan_new.h:288:1: note: Nan::imp::FactoryBase<v8::Number>::return_t Nan::New(double)
 New(double value) {
 ^
../node_modules/nan/nan_new.h:294:1: note: Nan::imp::MaybeFactoryBase<v8::String>::return_t Nan::New(const string&) <near match>
 New(std::string const& value) {  // NOLINT(build/include_what_you_use)
 ^
../node_modules/nan/nan_new.h:294:1: note:   no known conversion for argument 1 from ‘SQLINTEGER {aka long int}’ to ‘const string& {aka const std::basic_string<char>&}’
../node_modules/nan/nan_new.h:312:1: note: Nan::imp::MaybeFactoryBase<v8::String>::return_t Nan::New(const char*) <near match>
 New(const char * value) {
 ^
../node_modules/nan/nan_new.h:312:1: note:   no known conversion for argument 1 from ‘SQLINTEGER {aka long int}’ to ‘const char*’
../node_modules/nan/nan_new.h:318:1: note: Nan::imp::MaybeFactoryBase<v8::String>::return_t Nan::New(const uint16_t*) <near match>
 New(const uint16_t * value) {
 ^
../node_modules/nan/nan_new.h:318:1: note:   no known conversion for argument 1 from ‘SQLINTEGER {aka long int}’ to ‘const uint16_t* {aka const short unsigned int*}’
../node_modules/nan/nan_new.h:324:1: note: Nan::imp::MaybeFactoryBase<v8::String>::return_t Nan::New(v8::String::ExternalStringResource*) <near match>
 New(v8::String::ExternalStringResource * value) {
 ^
../node_modules/nan/nan_new.h:324:1: note:   no known conversion for argument 1 from ‘SQLINTEGER {aka long int}’ to ‘v8::String::ExternalStringResource*’
../node_modules/nan/nan_new.h:330:1: note: Nan::imp::MaybeFactoryBase<v8::String>::return_t Nan::New(Nan::ExternalOneByteStringResource*) <near match>
 New(ExternalOneByteStringResource * value) {
 ^
../node_modules/nan/nan_new.h:330:1: note:   no known conversion for argument 1 from ‘SQLINTEGER {aka long int}’ to ‘Nan::ExternalOneByteStringResource* {aka v8::String::ExternalOneByteStringResource*}’
odbc_bindings.target.mk:102: recipe for target 'Release/obj.target/odbc_bindings/src/odbc.o' failed
santigimeno commented 8 years ago

ping? :)

wankdanker commented 8 years ago

Hey @santigimeno,

Would you mind changing the declaration of i on line 822 (https://github.com/santigimeno/node-odbc/blob/32b3912400c2424ad5905350f05bd02c87c2ec6a/src/odbc.cpp#L822) to uint32_t and then remove the cast that you added?

If you can do that and it works, then I'll get that merged for you.

santigimeno commented 8 years ago

Sure no problem. In an initial version of the patch I had tried that approach and worked for me.

santigimeno commented 8 years ago

@wankdanker with the uint32_t change a warning is received:

../src/odbc.cpp: In static member function ‘static v8::Local<v8::Object> ODBC::GetSQLError(SQLSMALLINT, SQLHANDLE, char*)’:
../src/odbc.cpp:846:17: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for (i = 0; i < statusRecCount; i++){

Declaring it as a int32_t compiles just fine. WDT?

wankdanker commented 8 years ago

Yeah, we should use int32_t.

santigimeno commented 8 years ago

@wankdanker PR updated. Thanks!

wankdanker commented 8 years ago

odbc@1.2.1 published to npm.

Thanks for your help.

santigimeno commented 8 years ago

Thanks!