[sword-devel] SVN 3813 makes compilation of Sword fail

Mon Oct 19 03:55:55 EDT 2020

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 
>> <jaak at 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 at crosswire.org
>>>>> http://crosswire.org/mailman/listinfo/sword-devel
>>>>> Instructions to unsubscribe/change your settings at above page
>>>> _______________________________________________
>>>> sword-devel mailing list: sword-devel at crosswire.org
>>>> http://crosswire.org/mailman/listinfo/sword-devel
>>>> Instructions to unsubscribe/change your settings at above page
>>>
>>> _______________________________________________
>>> sword-devel mailing list: sword-devel at crosswire.org
>>> http://crosswire.org/mailman/listinfo/sword-devel
>>> Instructions to unsubscribe/change your settings at above page
>>
>>
>> _______________________________________________
>> sword-devel mailing list: sword-devel at crosswire.org
>> http://crosswire.org/mailman/listinfo/sword-devel
>> Instructions to unsubscribe/change your settings at above page
>>
> 
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page