[rfc][icedtea-web] C++-side small fixes (RH 1121549)
Jiri Vanek
jvanek at redhat.com
Thu Jul 24 14:14:32 UTC 2014
...
>> 1. Defect type: RESOURCE_LEAK
>> 10. icedtea-web-1.5/plugin/icedteanp/IcedTeaPluginUtils.cc:1154:leaked_handle – Handle variable
>> "plugin_file_log_fd" going out of scope leaks the handle.
>
> Does this one matter? I added a close() anyway but like Omair says it seems like it will be cleaned
> up anyway. But it's better to explicitly clean it I suppose.
Yes. As Omair pointed. Remove this close.
>
>>
...
>>> }
>>> return r2;
>>
>>
>> For those 4 mkdirs - if this directory already exists then the mkdir should not be called at all
>> (maybe some check for its write ability?? moreover 100% for different changeset)
>> Anyway. THis check is included. So I woud liek to add
>> (!IcedTeaPluginUtilities::file_exists(r1){} if (!isWriteableDirectory(){"warning to user}
>>
>> As thsoe are called only when and it do not exists, then more serious warring with possible
>> workarounds like check permissions on parents, set XDG_CONFIG* for first hunk and.. not sure what
>> for second :) or similar and warning like ITW will not work., SHould be printed.
>>
>
> So all of this in a separate changeset right? Can't most of this work be done just by checking errno
> after mkdir fails?
But please, Don't forget it. Othewrwise this check will lost a bit of sense.
>
>>
>>> }
>>> diff --git a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
>>> b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
>>> --- a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
>>> +++ b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
>>> @@ -704,7 +704,7 @@ IcedTeaScriptableJavaObject::setProperty
>>> browser_functions.intfromidentifier(name_id) >= 0) // else if array and requesting index
>>> {
>>>
>>> - JavaResultData* java_result = java_request.getArrayLength(instance_id);
>>> + java_result = java_request.getArrayLength(instance_id);
>>> if (java_result->error_occurred)
>>> {
>>> PLUGIN_ERROR("ERROR: Couldn't fetch array length\n");
>>>
>>
>>
>> I have looked into code and have not found a reason for in-the-middle-decklaration. So this is
>> really probably fix for hidden minor bug.
>>
>>
>> J.
>
> Yea, I couldn't find a reason for it to. Seems like it was probably just an accident.
>
After removal of close, ok for head and 1.5 from my side. Please verify with omair. He had his issues.
J.
More information about the distro-pkg-dev
mailing list