Nevermind,
We do exactly what you suggest in CURLFTPTransport and pass the stack
variable address. I've changed FTPLibFTPTransport per your suggestion
for parity with CURLFTPTransport.
Thank you,
Troy
On 10/19/20 11:25 AM, Troy A. Griffitts wrote:
Thanks Jaak,
I think it is safe to assume that the count of open files for our
process will be less than the storage capacity of a pointer. I can't
imagine an implementation where that has ever not been true or might
ever become false, but your solution would certainly work as well. As
for my preference to keep things as they are, it is simply a
preference: I don't typically like sending pointers of stack varables
off anywhere. For me, it's a good habit to keep me from times it
would be dangerous, though, as you noted, your suggestion would work
just fine here, as we are sure FTPLib blocks and doesn't let our stack
variable out of scope or else our call to close at the end of the
method would really mess us up! Thank you for the suggestion, as
always. I hope you've had a good weekend.
Troy
On 10/19/20 9:55 AM, Jaak Ristioja wrote:
On 19.10.20 10:22, Jaak Ristioja wrote:
Ah indeed, thanks for correcting my incorrect reasoning I did late
last night! But you are still relying on implementation-defined
behavior here which might not work for every platform, and there
might not even be an explicit guarantee it will continue to work for
your platform.
As far as I can tell, the safest way is to pass to the callback the
pointer to int. I think it would be valid in
FTPLibFTPTransport::getURL() to pass &fd, e.g.:
FtpOptions(FTPLIB_CALLBACK_WRITERARG, (long)fd, ftpConnection);
Sorry, I meant:
FtpOptions(FTPLIB_CALLBACK_WRITERARG, (long)&fd, ftpConnection);
Although according to
https://www.mbpfaus.net/~pfau/ftplib/FtpOptions.html "New programs
should call FtpSetCallback() and FtpClearCallback() to change
callback options."
and in my_filewriter() just convert the (void*) argument to (int*)
and, and dereference the pointer:
int output = *static_cast<int *>(fd);
I believe this would avoid the implementation-defined behavior. As
far as I can tell, the my_filewriter() callback is only called when
inside the getURL() function, hence fd is always reachable and valid
when my_filewriter() is called by FTPLib.
Best regards,
J
On 19.10.20 09:32, Troy A. Griffitts wrote:
Hi Jaak,
We're not storing a pointer here. FTPLib gives us a void * we can
do whatever we want with which we give them on initialization of a
file transfer and they simply pass this back to us on write calls
and status updates. We're storing the file handle there and we know
it's an int and thus we explicitly cast it to an int. I don't mind
a warning here, even with the explicit cast (which I assumed would,
and I think should, skip the warning) but the compiler actually
throws a compile error here, which is just stupid. I had to first
cast the pointer to a size_t and then to an int to avoid the
compiler error.
On October 19, 2020 1:07:48 AM GMT+02:00, Jaak Ristioja
<j...@ristioja.ee> wrote:
"Added extra cast (int)(size_t) to avoid stupid clang error that
doesn't
like void * being cast (int) directly to an int."
UH-OH!!!
First of all, the build that failed for BibleTime was using GCC not
Clang.
Secondly, the compiler is correct to warn, because a pointer does not
always fit into an int, e.g. for the LLP64 an LP64 data models
[1]. So
it seems that you might be throwing away half of the bits and
expect it
to always work. For those data models it might work if your OS only
gives you addresses in the range of [0, 2^32) but that is not always
guaranteed, leading to undefined behavior.
In short, please don't store pointers in integers other than intptr_t
and uintptr_t and convert them using reinterpret_cast! See also:
http://eel.is/c++draft/expr.reinterpret.cast#5
http://eel.is/c++draft/basic.stc.dynamic.safety
Best regards,
J
[1] https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models
On 19.10.20 01:23, Troy A. Griffitts wrote:
Crapola. Should be fixed. This was intended to be a call to the
new
FileMgr::write which hides the OS-specific impl, but I was
configured
to
use CURL instead of FTPLib, so I missed the compilation error. The
Android port uses FTPLib and I just built there successfully with
the
committed I just pushed. Thank you Jaak.
Troy
On 10/18/20 11:57 PM, Jaak Ristioja wrote:
Hello!
The commit "A bit more work on making it easier to use SWORD in a
threadsafe manner." makes compilation of Sword fail:
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’
loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’
loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in
this
scope
write(output, buffer, size);
^~~~~
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in
this
scope
write(output, buffer, size);
^~~~~
src/mgr/ftplibftpt.cpp:53:3: note: suggested alternative: ‘fwrite’
write(output, buffer, size);
See https://github.com/bibletime/bibletime/runs/1272250545 for the
failing build run.
Commit details in our git mirror of the Sword SVN repository:
https://github.com/bibletime/crosswire-sword-mirror/commit/c52559ecae
Best regards,
J
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
