Am 06/06/2023 um 11:41 schrieb Dominik Csapak:
>>> +my $untaint_recursive;
>>
>> I got flash backs w.r.t. refcount cycles here keeping all variables, and
>> thus memory
>> inside the body alive forever, don't we need a weaken?
>>
>> E.g., like we had to do in PVE::Status::Graphite's assemble.
>
> mhmm isn't that because there we use variables from outside the
> function? here we only use the parameters themselves
I'm not 100% sure about the details, but since then, seeing something like
this pattern triggers my cycle instincts, I'd like to have that checked out
closely.
>
> anyway the solution there is to set the sub to undef after use, but
> we can do that here only if we move the sub into the regular
> function
>
> i can also make it a proper sub if that's better?
>
> how can i test for these things properly?
easiest: pass large hashes and loop, then see if memory goes up. For metrics
back then you could see the RSS of pbvestatd grow by ~ the metric data size
every update.
What I also used back then, IIRC, was the Devel::Cycle module, it should give
you a more specific answer if there's a cycle, but naturally has no idea what
the practical implications are.
>>> +# convert arrays to strings where we expect a '-list' format and convert
>>> scalar
>>> +# values to arrays when we expect an array (because of www-form-urlencoded)
>>> +#
>>> +# only on the top level, since www-form-urlencoded cannot be nested anyway
>>> +#
>>> +# FIXME: change gui/api calls to not rely on this during 8.x, mark the
>>> +# behaviour deprecated with 9.x, and remove it with 10.x
>>> +my $convert_params = sub { my ($param, $schema) = @_;
>>
>> please keep the method paramethers on it's own line.
>
> oops, one shift+j to many without noticing^^
>
>>
>> Also, maybe go for a more telling names, as convert_params could mean
>> everytrhing
>> and nothing ^^
>>
>
> sure, any suggestions? 😉
sure, lets start with what this actually does in more explicit steps:
1) normalize_form_data
still a bit general but we now know that it handles form data and normalizes it
to a
single representation
2) normalize_param_list_to_array
A bit more telling about what happens.
3) convert_legacy_list_format_to_array
very telling, but as this is a internal helper, and thus not used elsewhere,
that
wouldn't hurt, having "legacy" in it underlines that we want to drop it
sometimes.
Personally, I'd favor 3) or 2).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
