[rfc][icedtea-web] C++-side small fixes (RH 1121549)

Jiri Vanek jvanek at redhat.com
Mon Jul 28 15:14:15 UTC 2014


> diff --git a/plugin/icedteanp/IcedTeaPluginUtils.cc b/plugin/icedteanp/IcedTeaPluginUtils.cc
> --- a/plugin/icedteanp/IcedTeaPluginUtils.cc
> +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc
> @@ -59,6 +59,19 @@
>   /* Plugin async call queue */
>   static std::vector< PluginThreadCall* >* pendingPluginThreadRequests = new std::vector< PluginThreadCall* >();
>
> +int
> +IcedTeaPluginUtilities::createLogDirectory(std::string dir)
> +{
> +	if (!IcedTeaPluginUtilities::file_exists(dir))
> +	{
> +		if (!g_mkdir(dir.c_str(), 755))
> +		{
> +			return errno;
> +		}
> +	}
> +	return 0;
> +}
> +
...
 > +		int parentErr = IcedTeaPluginUtilities::createLogDirectory(r1);
 > +		if (!parentErr){
 > +			PLUGIN_ERROR("FATAL: Failed to create IcedTea-Web config directory %s: %s\n", r1.c_str(), strerror(parentErr));
 > +			return NULL;
  		}

When you are her, you should also verify (if it exists then) whether it is directory or file. If File, return failure.

I would discourage you from returning errno.  I would say Print error messages here, and return true/false.

true/false is the only indicator you need if you need to decide wheter continue or not.

And I would suggest continue at all cost, but print out a warning that it probably will nto work.

So I would go with:

bool IcedTeaPluginUtilities::createLogDirectory(std::string dir)
{
if (!file_exists(dir) )
	{
		if (!g_mkdir(dir.c_str(), 755))
			int x=errno
			echo "warning, creation of $dir failed. Itw need this direcotry to work. You can expect failre later. reason: "error_toString(x)
//always save errorn
			return false;
		}
	} else {
		if (!is_dir(dir)){
			echo "warning, cannot create  $dir  becasue there is already an file"
			return false
		}
	}
	return true;
//feel free to add some debug only outputs here like exists, continuing, is dir, continuing...
}
int parentErr = IcedTeaPluginUtilities::createLogDirectory(r1);
// warnings have been printed
// no need to die, trying to continue




Also.. :-/ .. your patch kills ITW at my machine

[unknown user][ITW-C-PLUGIN][MESSAGE_DEBUG][Mon Jul 28 16:11:03 CEST 2014][/home/jvanek/hg/icedtea-web/plugin/icedteanp/IcedTeaParseProperties.cc:141] ITNPP Thread# 139757752895360, gthread 0x7f1bd2614120: FATAL: Failed to create IcedTea-Web config directory /home/jvanek/.config/icedtea-web: Success

and firefox dies.  Please run your code on soem usecases before posting.

Well the /home/jvanek/.config/icedtea-web do exist at my pc - so somethig is wrong :)


My reason to try continuing is simple - There may be cases, when .confing/itw may be some special device - well nearly impossible, but still possible.


Another note - sinc you created new method - please unittest it in cpp-unit-tests.


And positive note at the end. I run covescan on your patch and not sure what to read - http://cov01.lab.eng.brq.redhat.com/covscanhub/task/13258/ :))
J.





More information about the distro-pkg-dev mailing list