Full_Name: Ignacio Casal Quinteiro Version: OS: Windows 7 URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (2a01:111f:9fa:7f00:864b:f5ff:fe34:ddaa) https://github.com/nice-software/lmdb/commit/076281a2c03356667fca7cce1a898a4f155e3dce
nacho.resa@gmail.com wrote: > Full_Name: Ignacio Casal Quinteiro > Version: > OS: Windows 7 > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (2a01:111f:9fa:7f00:864b:f5ff:fe34:ddaa) > > > https://github.com/nice-software/lmdb/commit/076281a2c03356667fca7cce1a898a4f155e3dce > > I see no error when compiling with gcc -Wall. Closing this ITS. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
changed state Open to Closed
I am ok not fixing the MSVC warnings because it is picky even though some of them might make sense even for mingw, but in this case it is an error and this really small change enable msvc to build without errors. Can't you reconsider the change? Specially since it will build fine with mingw as well. On Sat, Oct 17, 2015 at 7:54 PM, Howard Chu <hyc@symas.com> wrote: > nacho.resa@gmail.com wrote: > >> Full_Name: Ignacio Casal Quinteiro >> Version: >> OS: Windows 7 >> URL: ftp://ftp.openldap.org/incoming/ >> Submission from: (NULL) (2a01:111f:9fa:7f00:864b:f5ff:fe34:ddaa) >> >> >> >> https://github.com/nice-software/lmdb/commit/076281a2c03356667fca7cce1a898a4f155e3dce >> >> >> I see no error when compiling with gcc -Wall. Closing this ITS. > > -- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/ > -- Ignacio Casal Quinteiro
Ignacio Casal Quinteiro wrote: > I am ok not fixing the MSVC warnings because it is picky even though some of > them might make sense even for mingw, but in this case it is an error and this > really small change enable msvc to build without errors. Can't you reconsider > the change? Specially since it will build fine with mingw as well. No, since it will break Unix builds. > > On Sat, Oct 17, 2015 at 7:54 PM, Howard Chu <hyc@symas.com > <mailto:hyc@symas.com>> wrote: > > nacho.resa@gmail.com <mailto:nacho.resa@gmail.com> wrote: > > Full_Name: Ignacio Casal Quinteiro > Version: > OS: Windows 7 > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (2a01:111f:9fa:7f00:864b:f5ff:fe34:ddaa) > > > https://github.com/nice-software/lmdb/commit/076281a2c03356667fca7cce1a898a4f155e3dce > > > I see no error when compiling with gcc -Wall. Closing this ITS. > > -- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/ > > > > > -- > Ignacio Casal Quinteiro -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
I do not understand, I tested also with mingw64 and here is the result I get with my patches applied: $ make gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mdb.c mdb.c: In function 'mdb_strerror': cc1.exe: warning: function may return address of local variable [-Wreturn-local-addr] mdb.c:1453:7: note: declared here char buf[1024], *ptr = buf; ^ gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c midl.c ar rs liblmdb.a mdb.o midl.o C:\msys64\mingw64\bin\ar.exe: creating liblmdb.a gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -fPIC -c mdb.c -o mdb.lo mdb.c:1:0: warning: -fPIC ignored for target (all code is position independent) /** @file mdb.c ^ mdb.c: In function 'mdb_strerror': cc1.exe: warning: function may return address of local variable [-Wreturn-local-addr] mdb.c:1453:7: note: declared here char buf[1024], *ptr = buf; ^ gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -fPIC -c midl.c -o midl.lo midl.c:1:0: warning: -fPIC ignored for target (all code is position independent) /** @file midl.c ^ gcc -pthread -shared -o liblmdb.so mdb.lo midl.lo gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mdb_stat.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mdb_stat.o liblmdb.a -o mdb_stat gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mdb_copy.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mdb_copy.o liblmdb.a -o mdb_copy gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mdb_dump.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mdb_dump.o liblmdb.a -o mdb_dump gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mdb_load.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mdb_load.o liblmdb.a -o mdb_load gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mtest.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mtest.o liblmdb.a -o mtest gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mtest2.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mtest2.o liblmdb.a -o mtest2 gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mtest3.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mtest3.o liblmdb.a -o mtest3 gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mtest4.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mtest4.o liblmdb.a -o mtest4 gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -c mtest5.c gcc -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized mtest5.o liblmdb.a -o mtest5 On Mon, Oct 19, 2015 at 8:35 AM, Howard Chu <hyc@symas.com> wrote: > Ignacio Casal Quinteiro wrote: > >> I am ok not fixing the MSVC warnings because it is picky even though some >> of >> them might make sense even for mingw, but in this case it is an error and >> this >> really small change enable msvc to build without errors. Can't you >> reconsider >> the change? Specially since it will build fine with mingw as well. >> > > No, since it will break Unix builds. > >> >> On Sat, Oct 17, 2015 at 7:54 PM, Howard Chu <hyc@symas.com >> <mailto:hyc@symas.com>> wrote: >> >> nacho.resa@gmail.com <mailto:nacho.resa@gmail.com> wrote: >> >> Full_Name: Ignacio Casal Quinteiro >> Version: >> OS: Windows 7 >> URL: ftp://ftp.openldap.org/incoming/ >> Submission from: (NULL) (2a01:111f:9fa:7f00:864b:f5ff:fe34:ddaa) >> >> >> >> https://github.com/nice-software/lmdb/commit/076281a2c03356667fca7cce1a898a4f155e3dce >> >> >> I see no error when compiling with gcc -Wall. Closing this ITS. >> >> -- >> -- Howard Chu >> CTO, Symas Corp. http://www.symas.com >> Director, Highland Sun http://highlandsun.com/hyc/ >> Chief Architect, OpenLDAP http://www.openldap.org/project/ >> >> >> >> >> -- >> Ignacio Casal Quinteiro >> > > > -- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/ > -- Ignacio Casal Quinteiro
On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: > I do not understand, I tested also with mingw64 (...) There's no LPTHREAD_START_ROUTINE type on non-Windows machines. You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' in the _WIN32 variant of '#define THREAD_CREATE...'. However, it's still wrong to call a function via another prototype. Maybe that'd fall on 32-bit Windows or something, I don't know how LPTHREAD_START_ROUTINE is defined. The safe way would be to give mdb_env_copythr() a prototype which does not need a cast. There is some code for that already. Maybe it can be improved. -- Hallvard
Sorry! I did indeed meant to change just the windows part, for some reason I thought it was already ifdeffed that part. https://github.com/nice-software/lmdb/commit/42577fe173923c1166f8382a53523a31479f9d43 This is the right patch. On Mon, Oct 19, 2015 at 1:43 PM, Hallvard Breien Furuseth < h.b.furuseth@usit.uio.no> wrote: > On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: > >> I do not understand, I tested also with mingw64 (...) >> > > There's no LPTHREAD_START_ROUTINE type on non-Windows machines. > You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' > in the _WIN32 variant of '#define THREAD_CREATE...'. > > However, it's still wrong to call a function via another prototype. > Maybe that'd fall on 32-bit Windows or something, I don't know > how LPTHREAD_START_ROUTINE is defined. The safe way would be to > give mdb_env_copythr() a prototype which does not need a cast. > There is some code for that already. Maybe it can be improved. > > -- > Hallvard > -- Ignacio Casal Quinteiro
So I took another closer look at this issue and I went for this possible solution: https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 Basically it errors out because MSVC needs stdcall on that method. Cheers. On Mon, Oct 19, 2015 at 2:55 PM, Ignacio Casal Quinteiro < nacho.resa@gmail.com> wrote: > Sorry! I did indeed meant to change just the windows part, for some reason > I thought it was already ifdeffed that part. > > https://github.com/nice-software/lmdb/commit/42577fe173923c1166f8382a53523a31479f9d43 > > This is the right patch. > > On Mon, Oct 19, 2015 at 1:43 PM, Hallvard Breien Furuseth < > h.b.furuseth@usit.uio.no> wrote: > >> On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: >> >>> I do not understand, I tested also with mingw64 (...) >>> >> >> There's no LPTHREAD_START_ROUTINE type on non-Windows machines. >> You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' >> in the _WIN32 variant of '#define THREAD_CREATE...'. >> >> However, it's still wrong to call a function via another prototype. >> Maybe that'd fall on 32-bit Windows or something, I don't know >> how LPTHREAD_START_ROUTINE is defined. The safe way would be to >> give mdb_env_copythr() a prototype which does not need a cast. >> There is some code for that already. Maybe it can be improved. >> >> -- >> Hallvard >> > > > > -- > Ignacio Casal Quinteiro > -- Ignacio Casal Quinteiro
nacho.resa@gmail.com wrote: > --047d7bb03a5ac118d20522842ead > Content-Type: text/plain; charset=UTF-8 > > So I took another closer look at this issue and I went for this possible > solution: > https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 That looks OK. > Basically it errors out because MSVC needs stdcall on that method. You realize of course, that this change actually has no effect on the code here? stdcall uses Pascal argument order instead of C order but it's moot since the function only has 1 argument. > > Cheers. > > On Mon, Oct 19, 2015 at 2:55 PM, Ignacio Casal Quinteiro < > nacho.resa@gmail.com> wrote: > >> Sorry! I did indeed meant to change just the windows part, for some reason >> I thought it was already ifdeffed that part. >> >> https://github.com/nice-software/lmdb/commit/42577fe173923c1166f8382a53523a31479f9d43 >> >> This is the right patch. >> >> On Mon, Oct 19, 2015 at 1:43 PM, Hallvard Breien Furuseth < >> h.b.furuseth@usit.uio.no> wrote: >> >>> On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: >>> >>>> I do not understand, I tested also with mingw64 (...) >>>> >>> >>> There's no LPTHREAD_START_ROUTINE type on non-Windows machines. >>> You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' >>> in the _WIN32 variant of '#define THREAD_CREATE...'. >>> >>> However, it's still wrong to call a function via another prototype. >>> Maybe that'd fall on 32-bit Windows or something, I don't know >>> how LPTHREAD_START_ROUTINE is defined. The safe way would be to >>> give mdb_env_copythr() a prototype which does not need a cast. >>> There is some code for that already. Maybe it can be improved. >>> >>> -- >>> Hallvard >>> >> >> >> >> -- >> Ignacio Casal Quinteiro >> > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
On Tue, Oct 20, 2015 at 10:09 PM, Howard Chu <hyc@symas.com> wrote: > nacho.resa@gmail.com wrote: > >> --047d7bb03a5ac118d20522842ead >> Content-Type: text/plain; charset=UTF-8 >> >> So I took another closer look at this issue and I went for this possible >> solution: >> >> https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 >> > > That looks OK. > > Basically it errors out because MSVC needs stdcall on that method. >> > > You realize of course, that this change actually has no effect on the code > here? stdcall uses Pascal argument order instead of C order but it's moot > since the function only has 1 argument. > yeah... but if this fixes the error I am fine with it, after all it end ups being just a define > > >> Cheers. >> >> On Mon, Oct 19, 2015 at 2:55 PM, Ignacio Casal Quinteiro < >> nacho.resa@gmail.com> wrote: >> >> Sorry! I did indeed meant to change just the windows part, for some reason >>> I thought it was already ifdeffed that part. >>> >>> >>> https://github.com/nice-software/lmdb/commit/42577fe173923c1166f8382a53523a31479f9d43 >>> >>> This is the right patch. >>> >>> On Mon, Oct 19, 2015 at 1:43 PM, Hallvard Breien Furuseth < >>> h.b.furuseth@usit.uio.no> wrote: >>> >>> On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: >>>> >>>> I do not understand, I tested also with mingw64 (...) >>>>> >>>>> >>>> There's no LPTHREAD_START_ROUTINE type on non-Windows machines. >>>> You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' >>>> in the _WIN32 variant of '#define THREAD_CREATE...'. >>>> >>>> However, it's still wrong to call a function via another prototype. >>>> Maybe that'd fall on 32-bit Windows or something, I don't know >>>> how LPTHREAD_START_ROUTINE is defined. The safe way would be to >>>> give mdb_env_copythr() a prototype which does not need a cast. >>>> There is some code for that already. Maybe it can be improved. >>>> >>>> -- >>>> Hallvard >>>> >>>> >>> >>> >>> -- >>> Ignacio Casal Quinteiro >>> >>> >> >> >> > > -- > -- Howard Chu > CTO, Symas Corp. http://www.symas.com > Director, Highland Sun http://highlandsun.com/hyc/ > Chief Architect, OpenLDAP http://www.openldap.org/project/ > -- Ignacio Casal Quinteiro
2015-10-20 23:09 GMT+03:00 <hyc@symas.com>: > nacho.resa@gmail.com wrote: > > --047d7bb03a5ac118d20522842ead > > Content-Type: text/plain; charset=UTF-8 > > > > So I took another closer look at this issue and I went for this possible > > solution: > > > https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 > > That looks OK. > > > Basically it errors out because MSVC needs stdcall on that method. > > You realize of course, that this change actually has no effect on the code > here? stdcall uses Pascal argument order instead of C order but it's moot > since the function only has 1 argument. > > No, there is another important difference: cdecl = the caller is responsible to clean stack from args (e.g. addl esp, N); stdcall = the callee is responsible to clean stack from args (e.g. retn N);
2015-10-20 23:09 GMT+03:00 <hyc@symas.com>: > > nacho.resa@gmail.com wrote: > > --047d7bb03a5ac118d20522842ead > > Content-Type: text/plain; charset=UTF-8 > > > > So I took another closer look at this issue and I went for this possible > > solution: > > https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 > > That looks OK. > > > Basically it errors out because MSVC needs stdcall on that method. > > You realize of course, that this change actually has no effect on the code > here? stdcall uses Pascal argument order instead of C order but it's moot > since the function only has 1 argument. > Oops, sorry (dummy gmail...). No, there is another important difference: cdecl = the caller is responsible to clean stack from args (e.g. addl esp, N); stdcall = the callee is responsible to clean stack from args (e.g. retn N);
Леонид Юрьев wrote: > 2015-10-20 23:09 GMT+03:00 <hyc@symas.com <mailto:hyc@symas.com>>: > > nacho.resa@gmail.com <mailto:nacho.resa@gmail.com> wrote: > > --047d7bb03a5ac118d20522842ead > > Content-Type: text/plain; charset=UTF-8 > > > > So I took another closer look at this issue and I went for this possible > > solution: > > > https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 > > That looks OK. > > > Basically it errors out because MSVC needs stdcall on that method. > > You realize of course, that this change actually has no effect on the code > here? stdcall uses Pascal argument order instead of C order but it's moot > since the function only has 1 argument. > No, there is another important difference: > cdecl = the caller is responsible to clean stack from args (e.g. addl esp, N); > stdcall = the callee is responsible to clean stack from args (e.g. retn N); True, but not relevant here. This is a ThreadProc, and threads never return to their caller. Windows implicitly calls ExitThread() when this function returns, and the stack will be destroyed anyway. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
fixed in mdb.master
changed notes changed state Closed to Test moved from Incoming to Build
Howard, any chance to get this in or should I change something else? Also, any chance you could have a look at "ITS#8274"? Thanks in advance! On Tue, Oct 20, 2015 at 10:37 PM, Ignacio Casal Quinteiro < nacho.resa@gmail.com> wrote: > > > On Tue, Oct 20, 2015 at 10:09 PM, Howard Chu <hyc@symas.com> wrote: > >> nacho.resa@gmail.com wrote: >> >>> --047d7bb03a5ac118d20522842ead >>> Content-Type: text/plain; charset=UTF-8 >>> >>> So I took another closer look at this issue and I went for this possible >>> solution: >>> >>> https://github.com/nice-software/lmdb/commit/03987789735141dc68ae8b2d0e5ad52d82cc80d0 >>> >> >> That looks OK. >> >> Basically it errors out because MSVC needs stdcall on that method. >>> >> >> You realize of course, that this change actually has no effect on the >> code here? stdcall uses Pascal argument order instead of C order but it's >> moot since the function only has 1 argument. >> > > yeah... but if this fixes the error I am fine with it, after all it end > ups being just a define > > >> >> >>> Cheers. >>> >>> On Mon, Oct 19, 2015 at 2:55 PM, Ignacio Casal Quinteiro < >>> nacho.resa@gmail.com> wrote: >>> >>> Sorry! I did indeed meant to change just the windows part, for some >>>> reason >>>> I thought it was already ifdeffed that part. >>>> >>>> >>>> https://github.com/nice-software/lmdb/commit/42577fe173923c1166f8382a53523a31479f9d43 >>>> >>>> This is the right patch. >>>> >>>> On Mon, Oct 19, 2015 at 1:43 PM, Hallvard Breien Furuseth < >>>> h.b.furuseth@usit.uio.no> wrote: >>>> >>>> On 19. okt. 2015 08:57, nacho.resa@gmail.com wrote: >>>>> >>>>> I do not understand, I tested also with mingw64 (...) >>>>>> >>>>>> >>>>> There's no LPTHREAD_START_ROUTINE type on non-Windows machines. >>>>> You could replace 'start' with '(LPTHREAD_START_ROUTINE)(start)' >>>>> in the _WIN32 variant of '#define THREAD_CREATE...'. >>>>> >>>>> However, it's still wrong to call a function via another prototype. >>>>> Maybe that'd fall on 32-bit Windows or something, I don't know >>>>> how LPTHREAD_START_ROUTINE is defined. The safe way would be to >>>>> give mdb_env_copythr() a prototype which does not need a cast. >>>>> There is some code for that already. Maybe it can be improved. >>>>> >>>>> -- >>>>> Hallvard >>>>> >>>>> >>>> >>>> >>>> -- >>>> Ignacio Casal Quinteiro >>>> >>>> >>> >>> >>> >> >> -- >> -- Howard Chu >> CTO, Symas Corp. http://www.symas.com >> Director, Highland Sun http://highlandsun.com/hyc/ >> Chief Architect, OpenLDAP http://www.openldap.org/project/ >> > > > > -- > Ignacio Casal Quinteiro > -- Ignacio Casal Quinteiro
changed state Test to Closed