Would be interesting to see what strptime is returning and compare it with what should be returned.
1 Like
I have identified no issues with it so far, it works a lot better than the netservices backend. I will probably consider using it by default in the next haikuwebkit release.
You can’t specify a format so you can’t know what it will do. Why don’t we just fix strptime and strptime_l instead of thinking of workarounds? I thought the point of Haiku was we could do the right thing and fix the bugs where they are instead of hacking around them…
3 Likes
Well, you noted there were problems e.g. with displaying SSL errors in the README. Indeed if those are fixed we should use it by default.
Incidentally, what is preventing a new HaikuWebKit release?
1 Like
I haven’t set up an account to enable me to do a PR but I’ve looked at strptime. For the %a (day string) and %b (month string) the wrong min values are being set ABDAY_1 & ABMON_1 instead of DAY_1 & MON_1.
So nl_langinfo(min) is returning the wrong values.
I hope someone can check this out and make the changes as I don’t have time to set up and account at the moment.
In linux these values are enumerated in the order
abday … day … abmon … mon
In haiku the order is
day … abday … mon … abmon
3 Likes
Sure. At this point is probably easier to just download an ISO and do the test, but I think I should be able to go back in time with pkgman’s system states? Anyway if someone could try this before me then by all means do it.
“Laburando” is a little too much saying 
Back to what’s needed for Web+ to function correctly with netservices, implementing strptime_l doesn’t SEEM to be difficult (it’s strptime but getting the locale as an argument). The curl specific function is probably our best bet until that is done I guess (and maybe even after that is done?) And then there’s the BeOS heritage parsedate (supposing that works for this use case)… so many choices.
ABI changes that prevent the beta3 builders from building it, I think? beta3 is too old for me to remember what we changed this time.
See: #17884 (TMR: strftime conversion specifier failed during test) – Haiku (haiku-os.org)
Are you retesting your test code using Haiku R1/b3 (i.e. this was working before recent MUSL conversion change commit in dev).
I didn’t have the time yet, but I reported the bug yesterday and seems that @Grumpy found the cause for this, confirmed by @waddlesplash
Also, I saw that nl_langinfo_l is already implemented, so implementing strptime_l for our purposes related to cookies in Web+ should be easy and I might give it a try. I was thinking in moving all the current strptime code into a internal function, and then strptime and strptime_l just get the current locale from the system or take it from an argument, respectively and call that code. But since it’s such an important part of the system and maybe is being kept in sync with upstream MUSL I’m open to suggestions from real devs on how to do this.
1 Like
Maybe ask on musl development mailing list whether they would like strptime_l implemented, eventually with suggestions.
I took your suggestion and just sent an email to the list, we will see what they think about it.
@PulkoMandy or anyone working in Haikuwebkit… I’m taking a look at implementing the methods in NetworkStateNotifierHaiku.cpp and I think startObserving() should call our own start_watching_network but then… How do I receive those messages? Can I make NetworkStateNotifier inherit from BHandler, should I create a new class? Thanks!
1 Like
You can create a BHandler and attach it to be_app (using AddHandler). This way the messages end up being processed in the main app thread where it is safe to call WebKit functions.
I don’t know if you can change inheritance for that class, I would need to check if it’s defined in a Haiku specific .h file or in a file shared with other platforms.
1 Like
Thanks for this. So if I’m understanding correctly attaching the BHandler to be_app is what allows it to live past the scope of the function (the be_app takes ownership). If this is the case then I got the idea.
About strptime_l, from the mailing list and quoting musl maintainer Rich Felker:
There are some number of nonstandard *_l functions, some of which have
been requested for addition. My request on these has been for someone
to do a survey of how many there actually are, and whether it makes
sense to add them all or some well-characterized subset of them. I
don’t want to just inconsistently add one here and there, while
leaving others missing, and I don’t want to get in a situation where
we feel obligated to add a bunch of dubious interfaces just by
precedent.If you’d like to make such a list/count, I’m sure it would be
appreciated by others who have raised the topic before, and might end
up with the functions getting added.Short of that, the portable way to do locale_t-parameterized calls to
functions that don’t have their own *_l version is by calling
uselocale() before/after to save/swap and restore the original locale.
This operation costs essentially nothing and is a completely viable
way to do things.
And then, after someone pointed to the list, he does an argument about each function and ends up with:
strptime_l then seems like it can just be considered on its own
merits.
So good news everywhere, it seems. Makes sense to fix strptime (which is already on the bugtracker with a possible solution), call uselocale() as @pulkomandy suggested some comments before and if one day strptime_l gets added to musl just use it. I volunteer to do the uselocale() thing in HttpTime.cpp, fixing strptime is a little over my coding level now.
8 Likes
I hope we are not completely dependant on musl to decide to add or not add a function to the C library. If we need it we can add it ourselves in our C library.
Is using uselocale in this way thread-safe? I would expect it is not, and that’s why the _l versions exist.
1 Like
It is, actually; it changes only the current thread’s locale.
2 Likes
I sent a patch here for BHttpTime to switch the locale to POSIX (C) before calling strptime and reverting it back inmediately after it’s done. It compiles and I ran a small test program linked against it (basically a short version of what BNetworkCookie does: provide BHttpTime a string, parse it and then printf’s to see what we’ve got) to see if it caused a runtime exception and it seems to be alright here. Obviously the result is still wrong cause strptime itself is broken, and so the real test will come when that it’s done. As always, I appreciate suggestions, fixes and the like.
5 Likes
Is that really the way locales are supposed to work? it sounds like a horrible hack to me.
Is there no way to call the function or an equivalent one and simply pass it an arg what the context is for parsing instead of changing the locale?
Why not fix it the right way with Grumpy’s solution? https://discuss.haiku-os.org/t/webpositive-doesnt-keep-sessions-open-on-websites/12479/24
Because as explained in the previous comments, there are two separate problems. Grumpy found the cause for one, I even left the link for the bug report a few comments ago. That is going to get fixed as soon as someone writes the code.
Read the thread?
There is strptime_l but it’s nonstandard and not yet available in Haiku.
1 Like
What makes you think that I did not?
I searched for strptime_l before and it turned up the same page as the none _l variant. So I assumed it was about thread safety since that was talked about above.
If we can use it we totally should even if it is nonstandard, posix only documents stuff already in use anyway 