ChanServ changed the topic of #asahi-dev to: Asahi Linux: porting Linux to Apple Silicon macs | Non-development talk: #asahi | General development | GitHub: https://alx.sh/g | Wiki: https://alx.sh/w | Logs: https://alx.sh/l/asahi-dev
linuxgemini9 has quit []
CME_ has quit [Ping timeout: 480 seconds]
CME has joined #asahi-dev
psykose has joined #asahi-dev
glem7 has joined #asahi-dev
timokrgr has joined #asahi-dev
glem has quit [Ping timeout: 480 seconds]
glem7 is now known as glem
chadmed_ has quit [Remote host closed the connection]
zzywysm has joined #asahi-dev
jeisom has joined #asahi-dev
jeisom has quit [Remote host closed the connection]
jeisom has joined #asahi-dev
Bertrand___ has quit [Ping timeout: 480 seconds]
Bertrand___ has joined #asahi-dev
xal has joined #asahi-dev
jeisom has quit [Remote host closed the connection]
jeisom has joined #asahi-dev
linuxgemini9 has joined #asahi-dev
linuxgemini9 has quit []
linuxgemini9 has joined #asahi-dev
KxCORP5 has quit [Quit: Bye!]
KxCORP5 has joined #asahi-dev
jeisom has quit [Remote host closed the connection]
<chadmed_>
for context upstream want the disk loading api implemented in a very specific (and imo silly) way and this is necessary based on suggestions made in MR 628
<chadmed_>
and if we dont have this fixed by approximately now then its not going to make it into a release before fedora 40 drops
<chadmed_>
its just this one call, the commits above it make their calls to wp_properties_get() just fine
<chadmed_>
commit 5db7a43a now :p
i509vcb has quit [Quit: Connection closed for inactivity]
<mixi>
chadmed_: not sure about why the 5db7a43a one segfaults, but you are not assigning to as_section in 02786b712d02
<chadmed>
mixi: well thats embarrassing :p thanks
<chadmed>
hmph its glib clearing the pointer to the object
<leio>
doesn't as_section leak now in some cases? Though only if wp_properties_get returns something that needs freeing and I don't know which it is
<leio>
ah, no, if it's set, it gets passed on to section.name, so that's intended presumably, nvm
<leio>
though not the case if any of the error early returns happen
<leio>
if that is true, you could make it a g_autoptr and then go through g_steal_pointer when passing it on to section.name
<leio>
chadmed_
PaulFertser has quit [Ping timeout: 480 seconds]
<leio>
though some no_frags case in the same file that's used the same doesn't concern itself about it either, so I guess it might be fine in your case too?
<leio>
wp_properties_get is (transfer none), so, uh, nevermind :)
jeisom has joined #asahi-dev
<leio>
not sure it's OK to assign a string to section.name without owning it, though
<leio>
wp_spa_json_parse_string is (transfer full), so that path does own it; so if section gets deallocated, it frees the strict that the underlying hashmap in wp_properties stuff owns
jeisom has quit [Remote host closed the connection]
<leio>
casting away the const is sort of a give-away of problem, I think?
jeisom has joined #asahi-dev
<chadmed>
the original function also just assigns a string to section.name via wp_spa_json_parse_string(tmp)
<chadmed>
the only issue im having now is that the parser returns an empty json object to me
<chadmed>
aha ive narrowed it down to the lua api
<leio>
chadmed: the thing is that wpa_spa_json_parse_string gives you a char* that the caller will own (transfer full), while wp_properties_get gives you a string that you do not own (transfer none)
<leio>
so I THINK you need to g_strdup it instead of casting away the const
<chadmed>
hmm okay
<chadmed>
let me try and fix the lua side first
<leio>
otherwise you'll have a UAF (use after free) down the line when the section you made gets deleted and the wp_properties stuff accesses it, or e.g. properties get reloaded and the section is accessed
<leio>
tl;dr: I think you need `section.name = as_section ? g_strdup (as_section) : wp_spa_json_parse_string (tmp);`
<chadmed>
thats what i had initially without the strdup
<chadmed>
the issue there is that without guarding behind if (as_section) it will try to parse the first token as a key then look for a value off that key
<chadmed>
the idea is to not do that, and parse the entire file as a value with a specified key name in WpProperties
<leio>
and I assume it crashed before because you g_autoptr as_section while wp_properties_get doesn't hand a string that you own
<chadmed>
so we need to guard all that recursive crap underneath
<leio>
but I can't find the old revisions anymore (and it doesn't matter)
<leio>
sad thing that "(transfer full)", "(transfer none)" and so on doc markup doesn't really help with anything much in C side beyond being able to see it as a documentation to know what to do with it in C, but they are used to make bindings memory safe at least
<leio>
(as long as the documentation matches what the library C function actually does :D )
jeisom has quit [Remote host closed the connection]
<chadmed>
ahhh section.name has to be a parsed spa-json string -.-
<chadmed>
for some reason we can retrieve the named section as a parsed string value successfully, but trying to return it via push_luajson() causes it to misbehave
<chadmed>
while this doesnt matter for our use case since we need it to be a string anyway, it's not a very good general use API
nyaomixyz has joined #asahi-dev
mixi has quit [Quit: quit]
mixi has joined #asahi-dev
Tramtrist has quit [Remote host closed the connection]
Tramtrist has joined #asahi-dev
opticron has joined #asahi-dev
Tramtrist has quit [Remote host closed the connection]
Leo3418 has quit [Quit: Applying updates]
Leo3418 has joined #asahi-dev
Bertrand___ has quit [Read error: Connection reset by peer]