sanjeev has quit [Remote host closed the connection]
otaviobp has joined #soletta
anselmolsm has joined #soletta
spoussa has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
bdilly has joined #soletta
ceolin[AWAY] is now known as ceolin
glima has joined #soletta
glima has joined #soletta
ricardotk has joined #soletta
mbelluzzo has joined #soletta
spoussa has joined #soletta
caiortp has joined #soletta
mRse7enNIT has joined #soletta
mRse7enNIT has quit [Read error: Connection reset by peer]
kamidi_preetham has joined #soletta
spoussa has quit [Ping timeout: 260 seconds]
iscaro has joined #soletta
otaviobp has quit [Quit: WeeChat 1.5]
bsmelo has joined #soletta
<bsmelo>
hi guys! talking with barbieri yesterday he told me you can point me to a patched version of tinydtls which I can actually build with soletta (I would only replace the contents inside thirdparty/tinydtls)
<bsmelo>
could anybody point me to such repository, please?
<glima>
ceolin might know
<glima>
he's afk but should answer soon
<bsmelo>
np, thx!
<iscaro>
maybe otavio as well
<iscaro>
I guess he's not here
<bdilly>
bsmelo, hm... but we're keeping it private
<bdilly>
I don't think I would be able to give you permissions on this private repository
<bdilly>
give me some time to figure out a solution for that
<bdilly>
I've talked many times with iotivity guys about keeping these patches on tinydtls upstream...
<iscaro>
I still don't know why they did not upstream those patches yet
<bdilly>
lack of resources they say
<bdilly>
anyway, do you really need them?
<iscaro>
...
<bdilly>
afaik they are required for OIC
<bdilly>
do they impact lwm2m as well?
<bdilly>
iscaro, ^
<iscaro>
I don't know what kind of changes they made to tinydtls.
<bdilly>
hehehe
<iscaro>
I guess we don't need them
<bdilly>
so why do you need those?
<bsmelo>
the biggest part of my gsoc proposal is to be able to use lwm2m through encrypted coap, and I would use tinydtls for that...
<iscaro>
I mean the changes
<iscaro>
not tinydtls
<iscaro>
the patched version of tinydtls
<iscaro>
that's what I meant
<bdilly>
bsmelo, got it
<bsmelo>
yeah, but currently I can't build soletta with tinydtls (if I select it using menuconfig I get an error)
<iscaro>
yeah yeah
<iscaro>
I know :/
<Sachiel>
disable OIC?
<iscaro>
you can disable oic
<Sachiel>
or maybe the dtls socket uses things from those patches
<iscaro>
ouch
<bsmelo>
yeah, the error is inside the dtls socket impleemntation
<bdilly>
bsmelo, you could check the iotivity version
<bsmelo>
ok, I will try to build with that! thanks!
<bdilly>
ceolin, do you confirm that? ^ long time I don't build our oic implementation with dtls support
<bdilly>
bsmelo, thanks. let us know if it fails
<acidx>
bsmelo: regarding tinydtls, the changes that were made were required for the OIC protocol, not sure if that's required for lwm2m as well
<acidx>
bsmelo: that's mostly for new ciphers, and things that are used for the provisioning/pairing step of the protocol
<acidx>
bsmelo: you'll likely need to disable OIC though as the soletta implementation will reference new symbols not available in upstream tinydtls (but available in the repo bdilly linked you)
<iscaro>
acidx, it looks like that our sock impl depends on those changes
<acidx>
ah, yeah, fair enough. there are some sol-socket-tinydtls only functions that will enable those extra ciphers and whatnot
<gabrielschulhof>
Hey! glima is reviewing my PR (https://github.com/solettaproject/soletta/pull/2055) and I'm wondering about sol_ptr_vector ... When you remove a pointer from a sol_ptr_vector, does it call a function that frees the pointer?
<glima>
gabrielschulhof: nope
<gabrielschulhof>
glima: OK, so when you remove the pending_reply from the sol_ptr_vector, it does not get freed even now, right?
<glima>
gabrielschulhof: no, you free() it by hand afterwards (pending_reply_free())
<gabrielschulhof>
OK. Good.
<glima>
my point was reaching sol_ptr_vector_remove(vector, i) for an i that was long gone
<glima>
at the end this is harmless
<gabrielschulhof>
... because I've made sure that that code path is followed not only when the callback returns false, but also when it returns true and the pending_reply->freshness is set to PENDING_REPLY_FREE_ME.
<glima>
saw that
<gabrielschulhof>
So, it will remove from the sol_ptr_vector just as if the callback had returned false.
<gabrielschulhof>
So, does that not mean that it's no leakier than it was before?
<gabrielschulhof>
I just feel like I'm missing something ... sorry!
<glima>
i'm not reporting a leak
<glima>
just a r < 0 for sol_ptr_vector_remove() :)
<gabrielschulhof>
r?
<glima>
it's the opposite of leak, it's trying to remove more than once for the vector
<gabrielschulhof>
Oh, that's bad!
<glima>
gabrielschulhof: line 671
<glima>
not in your patch, but now impacted
<gabrielschulhof>
... because it's gonna remove another pointer it's not supposed to remove.
<gabrielschulhof>
OK.
<glima>
it will just fail looking for it
<glima>
and we don't check for r, so whatever
<glima>
but we could avoid it as well
<gabrielschulhof>
Why would that fail?
<gabrielschulhof>
Oh, wait, I get it!
<glima>
good :)
<gabrielschulhof>
... because the recursive call has already removed it.
<glima>
i was going to open the files all over again :)
<glima>
yeah
<gabrielschulhof>
OK, gotcha.
<glima>
:DH
<gabrielschulhof>
So, basically, the code for removing the pending_reply should be in one place and it should only be referenced if you're really, really gonna free the pending_reply.
<glima>
the rest seems to do it right
<glima>
just waiting for more eyes there
<gabrielschulhof>
OK, so the double free is gone, but the double-remove-from-the-vector is still there.
<glima>
thanks for spotting the issue
<glima>
gabrielschulhof: it will silently fail, as i said
<glima>
no big deal
<gabrielschulhof>
Yeah, in JS you do all kinds of crazy things because of the massive amount of asynchronicity, so stuff like this comes up even in pure-JS scenarios.
<glima>
someone might have an idea
<glima>
gabrielschulhof: that's good
<gabrielschulhof>
glima: Oh, I see, it's because you're removing by vector, not by index.
<glima>
yeah
<gabrielschulhof>
Oh yeah, that'll fail, unless you allocate a second pointer with the exact same value.
<iscaro>
babu_, ping
<glima>
on small oses if the planets align that might happen
<iscaro>
babu_, The UART PR is merged
<gabrielschulhof>
Which, crazy as it seems, might be the case if you issue a new request from the callback to the old request.
<glima>
mm
<gabrielschulhof>
No, actually, wait, it won't happen, because you only remove after that callback has already returned.
<gabrielschulhof>
So, you remove twice on your wait out of the stack, not on your way in.
<gabrielschulhof>
BUt hey! If we wanna be pedantic about it, we should make the removal happen iff(sic) we really free the pending_reply.
<glima>
yeah, now that you're on it, ideas accepted :)
<gabrielschulhof>
OK, so should I bring being pedantic into the scope of this PR?
<ceolin>
bdilly: bsmelo i guess so (about the repository). They were trying to upstream the patches but i'm not sure if it's already in
<glima>
i'd stay in the PR myself, no problem
<glima>
it's totally related
<gabrielschulhof>
OK.
<gabrielschulhof>
glima: The same might apply to sol-oic-client.c as well, come to think of it.
<glima>
yeah? which line?
<gabrielschulhof>
glima: Not sure. I have to check. But now that I know what to check for, I'll make sure that if sol-oic-client.c is also affected, then I'll be pedantic there as well :)
<gabrielschulhof>
glima: Actually, the exact same thing is happening one level above, in the node.js bindings.
<bsmelo>
bdilly: ceolin with iotivity's tinydtls I get errors in the linking stage for `LD build/soletta_sysroot/usr/lib/libsoletta.so`
<gabrielschulhof>
I was getting a segfault in the bindings precisely because I was freeing my own structures recursively.
<gabrielschulhof>
glima: ... but in the binding I have an easy job, because I have a pointer to a pointer which, if null, tells me it's already freed so I shouldn't free it a second time.
<bsmelo>
bdilly: ceolin the folder has a lot of .patch files and I'm checking them... but apparently they're all applied already
<glima>
gabrielschulhof: i see
<gabrielschulhof>
glima: Actually, I have an idea: If we have this sol_ptr_vector, I can check if the pending_reply is in it. If it's not, I don't need to free it. What do you think?
<ceolin>
bsmelo: yeah, those patches they are trying to upstream
<glima>
gabrielschulhof: isn't this what happens now?
<gabrielschulhof>
No.
<bsmelo>
ceolin: I guess I'll have to check one by one... but it seems they're all applied in this repo anyway
<gabrielschulhof>
Right now, I call pending_reply_free(), but pending_reply_free() doesn't actually free(), it just sets reply->freshness to PENDING_REPLY_FREE_ME and returns.
<gabrielschulhof>
Then, I remove it from the vector.
<gabrielschulhof>
... and return.
<ceolin>
bsmelo: afaik they are
<gabrielschulhof>
Finally, in the outermost frame, I call pending_reply_free() again, but because freshness is set to PENDING_REPLY_FREE_ME, it actually free()s the reply.
<gabrielschulhof>
... and I remove from the vector again.
<iscaro>
gabrielschulhof, look my comments
<iscaro>
at your PR
<gabrielschulhof>
iscaro: Cool! Thanks!
<gabrielschulhof>
iscaro: OK. I'll have a look and use the same pattern.
<bsmelo>
ceolin: all error are undefined references to dtls_*log_* symbols. do you have any idea what would help?
<iscaro>
gabrielschulhof, first check if the pattern solves the problem
<gabrielschulhof>
iscaro: Of course.
<iscaro>
;]
<gabrielschulhof>
iscaro: Either way, I have some leads for a v2 of the PR.
kamidi_preetham_ has joined #soletta
<gabrielschulhof>
iscaro, glima: Thanks for reviewing!
<iscaro>
np
<glima>
gabrielschulhof: :) let's wait for the new organisation/pattern to eval if we still have the vector issue
kamidi_preetham has quit [Ping timeout: 250 seconds]
<gabrielschulhof>
glima: Yeah, it's good that you helped me understand this issue. I now know what to look for.
<glima>
cool
spoussa has joined #soletta
bottazzini has quit [Remote host closed the connection]
spoussa has quit [Ping timeout: 260 seconds]
spoussa has joined #soletta
<gabrielschulhof>
iscaro: You're using the exact same mechanism, except you're using two booleans, whereas I'm using a tri-state integer :)
<iscaro>
:o)
<gabrielschulhof>
iscaro: In your case you're fortunate, because with sol_uart_close() you have all the closing code in one place. I need to similarly concentrate the teardown code for the pending_reply and for the OIC client pending handles and then only call it once when it's ready to go.
<iscaro>
Yeah
<iscaro>
I was refering to that as well
<gabrielschulhof>
Aaah, OK.
<iscaro>
it will be better to maintain in the future
<gabrielschulhof>
Yeah, definitely.
<gabrielschulhof>
This stuff tends to creep up on you. I bet it started with a simple free(), and then pending_reply_free(), and then that + remove from vector, and so on :)
<iscaro>
it's like a baby, one day you will have to let it go
<iscaro>
ahahaa
<ceolin>
lol
<glima>
lol
<gabrielschulhof>
ROTFL :)
spoussa has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
spoussa has joined #soletta
spoussa has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
bottazzini has joined #soletta
spoussa has joined #soletta
spoussa has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
spoussa has joined #soletta
spoussa has quit [Remote host closed the connection]
_whitelogger has joined #soletta
kamidi_preetham_ has quit [Ping timeout: 250 seconds]
caiortp has quit [Remote host closed the connection]
caiortp_ has quit [Remote host closed the connection]
mbelluzzo has quit [Quit: Leaving]
anselmolsm has quit [Remote host closed the connection]
bdilly has quit [Quit: bdilly]
ricardotk has quit [Quit: vlwflw]
bottazzini has quit [Remote host closed the connection]
bottazzini has joined #soletta
glima has quit [Remote host closed the connection]
ceolin is now known as ceolin[AWAY]
lvladi has quit [Remote host closed the connection]