[sword-devel] Transport status reporting

Wed Jul 31 07:08:29 MST 2013

Thanks for the report Jaak.  This is excellent information.  I 
appreciate you giving the benefit of the doubt to the original 
contributor and investigating why they might have made the method 
signature the way they did originally.  That was gracious; that you.

I've added the new method signature as you've suggested (slighted 
different to match the preStatus method), and deprecated the 
statusUpdate method.  Oddly GCC doesn't warn me that I've overloaded the 
deprecated method in the installmgr utility.  I'm calling the deprecated 
method from a default implementation of the new method, so if you don't 
override the new method (which most people won't yet), they'll still get 
their status updates.

Thanks for the heads up about the coming curl changes and for suggesting 
this interface addition before the release.  This will allow us to add 
the better support for curl in a 1.7.x release.

Troy

On 07/31/2013 12:09 PM, Jaak Ristioja wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi!
>
> Yesterday, I again found myself writing safety workarounds around
> Sword status reporting, i.e. for the
> sword::StatusReporter::statusUpdate() callback, which for some reason
> uses floating point doubles instead of integers. Digging my way into
> the depths of Sword I noticed that this was implemented this way
> probably due to libcurl providing similar callbacks with doubles.
> Looking at the libcurl source code, I noticed that for their upcoming
> version 7.32.0 they have implemented a better callback for progress.
> For reference, see
>
>    https://github.com/bagder/curl/commit/12d01cb6
>
> It should be possible to use some
>
>    #include <curl/curlver.h>
>    #if LIBCURL_VERSION_NUM >= 0x072000
>
> magic in Sword to provide similar callback alternatives for Sword
> programs. I suggest that the sword::StatusReporter class be extended
> with this method:
>
>    virtual void update(size_t dlTotal, size_t dlNow);
>
> keeping the older statusUpdate(double, double) as well. When progress
> is reported by the new curl callback, both update() and statusUpdate()
> should be called for backwards compatibility.
>
> If using curl 7.32.0 or later, for the update() call, curl_off_t
> arguments (signed integers of some width) are converted to size_t, and
> for statusUpdate() call they are converted to double().
>
> If using earlier versions of curl, for the update() call, double
> arguments are converted to size_t, and for statusUpdate() call they
> are passed as-is.
>
> As you can easily see, both old and new approaches in curl have its
> deficiencies, since both double and curl_off_t are both signed. An
> additional caveats come from conversions to or/and from doubles and
> operations on them, which might involve using the FPU (integer
> operations are much faster on CPU cores), might yield imprecise
> values, invalid values or floating point exceptions. The latter might
> just crash the program in some circumstances. I haven't time to
> formally analyze whether some or all of these issues affect BibleTime,
> but our code for statusUpdate() has workarounds in case the passed
> doubles are negative, if (dlTotal == 0.0), if (dlNow > dlTotal), etc.
> I guess we could also call fpclassify() etc to work around other
> corner cases, but in principle, this is not what frontend developers
> should be dealing with, so I suggest a proper update(size_t done,
> size_t total) interface be defined, where total > 0u and done <= total
> always hold.
>
> The old statusUpdate(double, double) interface should be deprecated.
>
> Blessings,
> Jaak
>
> PS: I'm not sure whether we should to delay the 1.7.0 release for
> this, although this issue might involve an API change.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.20 (GNU/Linux)
>
> iQgcBAEBAgAGBQJR+OHVAAoJEEqsYmEt1rCOY6Q//1LTeiEsfFdN8b/KtOzbFNWR
> xGpJ5HBP/Dxcbkxibhaw5fbbHUUqrnyq4MA5jtnazcGmhkPsuw6ZyMooBMmht27y
> tqNQOePPFh2//O+aScDbGJgoTWoVPvV6pbAx53UKwHPxNxTQObiH0nDt3B/cbT14
> bHIRDRfAKT0keINd8OZKQ6wizjrvsTbzOj3R52iYHfatIcIt1vrLT/gss4mWk0NO
> 6OAJx/3tUNetLb44wVEiB4ZRFUJMjreBWiM6/+Rhsmkttwuz1pvbGtW/yzvQEyls
> UschH8hBCKlyEkxzszR+g2HCD3c5EKDX/EASNbpEIo8xE8+DDi1nLgm3I2UmRY/8
> cjeoQNzzP+Pho8ug3fa2S/iJZ3Xey9xgv7xeelliC4ULLlhLC8hjQXZgJe0pwajq
> FqbyTPVmEfdvrQ4mc4KoNMHcHFnPZKf0nFi0k2Jt86lVtPIQnhnIpRM2rl4vzRiE
> dpm9zjaefXYZACPH2Jqh7dpktW/MwmHqqL5ziylXnitOUl4y8D8rEv6UKyDxtoeW
> XktmmaGFGgv8Ct/ZStnYoV7zIX972Mx+UHpPNm1iamUSQZAZNAFEMks2fw6CZ7Mf
> 5CRi/tqkYZg1Zj3w2Zp8NI+0KXSHOEG1OMrFvQNgRXIu3VW3CvNbLXpDEkuSANlR
> sdq1OH3/zbw7L+LusiMp5760VkKeOdfXam29fzfNkuVpp060OuECDhQkZYDoo2dB
> 6E4MTaBQjBjMjnusU2JnXgBkPvMQdNN6o9wY2cXGKYqneSmcgEekfvgvi5vRRiyW
> ZufHKh1MFwxSK/I5jQcHt9lq744vi65kMW84VPBmdW3DCwyePwFwBWzwHAZ4X087
> X62VJeKfoD025Ggj9+6niAhiW8Ue1sX3PIx5qx5n4hnc3oNkiaukfddo9JQXQ32M
> m+H2ukBtS4inFfXEbVmN/SCBDNOg7pC6H41clypXaKt40TpnXqWDheiyjEP39q+q
> UMYpaZKIdUVRrjQj5yiH6+kK8TDCneAfdFgiwfMV3/9lo211BG93+t/FTg5Yv5G+
> b6pNSBr7rw+rlfx2v2Eyey4j/b/eGLVAuRnuTRhNrgkO345k6uU8NMKtNunpQaAT
> H0eABPr2qRjZF69SnWs2Wjn5xWvmWcnG+i/IKsVExInSghfX2wHAJMYLszVsG9Cz
> er8wiLOj5gvccHa3JnJqOihEiiCADq+Tf0xXBw+Jr1TyAeuFgoAYOZEyv28tHucF
> QN83xOWAh69ELzptQ3ppFAyTFk6gyCizkNrmpPjnn3DCtyAByy/BzoObwpt1rAG8
> lr+DfKTNALUQrA0aN4Y/kT4e+LNPIb3um0PQ3CujnD3bfEsGfBeEaw+25edjsRWC
> thAvej8z2wHLFpidA1WpSy5rJ91ZRqdMPwiTs5oA8MfFrjqUKcLhto7+mk6VITAX
> 2WScQh4+9UDOEyLgs3BAz4/63gwhu8Ak6/GZNoDG+6LIDRV3waMQbGte36htIu7F
> SLgc1SqqjT0NW229RNVc+Vf9sS/xxZyCpPYtHUYlSrmwhlVQsKlspvvj2tyFG878
> JdslP/6jnCKHXOjhBb8OJroRzsKH+iQnMk/4hz5SQXtXTEIVleOHjtXb0CpwPOl9
> qABAhiPpyn4OoTTYDnzYFKun/9QMKCkJCdTmEm69jmhY8t2Rw07pgwK4otSvUFpY
> 3W4DVMW3FXU1EGrT25Tv2joP9kUyoEU9AnQRPYTxr98AMVQCb+fjhONSBYphhKVw
> P0HXYqMn/5qBEmL04xFe/TPFcp3CkRJ0XgZtYHPLYJoY5oy3wTtHtVx+KD6XKsx1
> +YE9C5U0uVLvA7XBGsOiaw2U8CwAQusVwgaId+IXkjfhBNhTewP1egZWtN7cN/Xb
> G93FQjHTcVrsKXN62COsOyhmBDgeOt+hAYhQfIJlaMQfwA7cpQKS+gkqCfAJpirt
> oX3VXcQS462LrDSMBdmjKfz4SmYPhh6tW69P5r/Fw63YGJGj8HZlTrziaEgs4V/q
> KjwY4xe0jGcKG+Qaau5cFQB6QS6cLnLfW0fUkNXkDFYkiO3r5++Ou0NmcHN/4ssB
> /7qOTursRDKK7wUpQPopxqf6GI+G+FyuJW0KuDtxm5w+taopN5TBZ/afi6kPHYNs
> ByKLk+nqN4D6Gh5QVwPLBL1cRGsaF19OwhC3K7d69lURZLYsBVWSVbs+PGar2ZtK
> 5sEkNA8Ule00mZcYzf6b7AHQIvsDjA34ur+clsX6g06K3Rf7j39YHg3zLL7lZfnB
> ieCv2TVI3/+xz9syyFGQ0V9yTwk+hbLndE9i0UA4vItFFEbNhNOfD7BmJ1XNveh1
> GNAi3N84OtQm0sScOLmFL1JSj0AHMq2+lsUosxw/4pMy5OS3GmlLTyaIhcOt6KS5
> S3bKjEmyBzcrX3emeKKDBt4OY8q8AEztMJ3lVXyGtU5fwSmFDtGE2tnG/gNmEy7k
> /jZ03Szf8KLJIdJTLbp8hHFzxodXNH3cM4hSYezY++gXTaWcdkpai54W3QcDnTdo
> Op6sFKsy9I23JTqkaUzkhV9rUJBVzCm4Gk+/1CdPs0XFkBuFj1t9PambqkbWjqIV
> KlCCf4IWNrs3G0XuVR+05WTKHgyWnu/pYB8Zx2vEn2ocOfFwYQf/UBaTWYszY9KB
> HpfDWMntITJNGLbJzymDdEigC/oHc1MdRm35LXOP85WlSXEAJ93aPAvfzjp1T78y
> m88Ta1IHVOCmLdGSxMSo
> =hokR
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page