Re: [POLICYREP] [RFC/PATCH 1/3] policy package class

[Karl MacMillan <kmacmillan@mentalrootkit.com>]

On Tue, 2007-07-17 at 11:03 -0400, method@manicmethod.com wrote:
> plain text document attachment (00-policy_package-header.patch)
> --
> 
> --- a/libpolicyrep/include/policyrep/policy_package.hpp	(revision 0)
> +++ b/libpolicyrep/include/policyrep/policy_package.hpp	(revision 0)
> @@ -0,0 +1,38 @@
> +/* Author: Joshua Brindle <method@manicmethod.com> */
> +
> +#ifndef __policy_package_hpp__
> +#define __policy_package_hpp__
> +
> +
> +#include <policyrep/policy.hpp>
> +
> +namespace policyrep
> +{
> +	struct PolicyPackageImpl;
> +	class PolicyPackage
> +	{
> +	public:
> +		PolicyPackage();
> +		PolicyPackage(std::string filename);
> +		~PolicyPackage();
> +

Destructors should always be virtual - otherwise subclassing is
difficult.

> +		const Module get_policy_module() const;
> +
> +		const std::string get_file_contexts() const;
> +		const std::string get_seusers() const;
> +		const std::string get_user_extra() const;
> +		const std::string get_netfilter_contexts() const;
> +

why not const std::string&? I also think we should always default to
virtual.

> +	protected:
> +		void init();
> +		int read_package(std::string filename);	
> +
> +	private:
> +		PolicyPackageImpl* impl;

We need to make a decision on whether this is going to be protected or
private and stick with that. I've done protected so far - if you want to
change it please say why and send a patch changing all existing classes.

> +	};
> +
> +	typedef boost::shared_ptr<PolicyPackage> PolicyPackagePtr;
> +

This may not be needed here - it is currently only definitely used for
objects that are part of the ast.

Karl

> +} // namespace policyrep
> +
> +#endif
> 


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.