-
Notifications
You must be signed in to change notification settings - Fork 806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pricedb to stl #1731
Pricedb to stl #1731
Conversation
b39e83f
to
599abfd
Compare
@christopherlam Re 2d730e3 and 6b741dd What's the point of having a PR--one marked Draft no less--when you're just going to push the commits directly to stable? BTW, I think the struct->std::pair change isn't a great idea on its own: It adds a bunch of overhead, and reduces expressiveness because "first" and "second" are less meaningful than "to" and "from". I didn't comment here because you have it marked draft and I was waiting for you to do something that would actually use some feature of the pair. |
Ok we can revert the |
599abfd
to
99cee35
Compare
e93c16a
to
5968e32
Compare
* use g_list_find_custom where appropriate * also g_list_free_full and g_list_foreach
5968e32
to
fc21c55
Compare
auto price_list = pricedb_get_prices_internal (db, c, currency, TRUE); | ||
if (!price_list) return NULL; | ||
|
||
for (auto item = price_list; item; item = item->next) | ||
auto p = g_list_find_custom (price_list, GUINT_TO_POINTER(t), (GCompareFunc)price_time64_less_or_equal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GUINT_TO_POINTER
is an evil thing that subverts the type system. There are two problems in this case: First of all, time64
s are signed and second they're int64_t. GUINT is an alias for uint32_t, and pointers on 32-bit builds (Windows!) are 32 bits. See the patch on bug 799104. The user doesn't explain themself well, but I guess they're doing forecasting and have dates past 2038 when time goes over 32 bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops.
first easy steps.