[redhat-lspp] Re: [RFC 2/7] NetLabel: core network changes

[Steve Grubb]

On Thursday 06 July 2006 17:34, paul.moore at hp.com wrote:
> +++ linux-2.6.17.i686-quilt/include/net/cipso_ipv4.h
> +int cipso_v4_doi_remove(const u32 doi,
> +			void (*callback) (struct rcu_head * head));
> +struct sk_buff *cipso_v4_doi_dump(const u32 doi, const size_t headroom);
> *domain); 

You do not need to have const on pass by value vars.

> +int cipso_v4_error(struct sk_buff *skb,
> +		   const int error,
> +		   const u32 gateway);

same

> Index: linux-2.6.17.i686-quilt/include/net/netlabel.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6.17.i686-quilt/include/net/netlabel.h
> +static inline size_t netlbl_align(const size_t length)
> +{
> +	return NLMSG_ALIGN(length);
> +}

unneeded const on pass by value

> +static inline void netlbl_put_u8(unsigned char *buffer, const u8 val)
> +{
> +	*(u8 *)buffer = val;
> +}

same

> +static inline void netlbl_put_u16(unsigned char *buffer, const u16 val)
> +{
> +	*(u16 *)buffer = val;
> +}

same

> +static inline void netlbl_put_u32(unsigned char *buffer, const u32 val)
> +{
> +	*(u32 *)buffer = val;
> +}

same

> +static inline void netlbl_put_str(unsigned char *buffer, const char *val)
> +{
> +	strcpy((char *)buffer, val);
> +}

why not just use strcpy to make it more obvious what this is doing?

> +static inline void netlbl_put_hdr(unsigned char *buffer,
> +				  const u32 msg_type,
> +				  const u16 msg_len,
> +				  const u16 msg_flags,
> +				  const u32 msg_pid,
> +				  const u32 msg_seq)
> +{

const on pass by value

> +static inline void netlbl_putinc_hdr(unsigned char **buffer,
> +				     const u32 msg_type,
> +				     const u16 msg_len,
> +				     const u16 msg_flags,
> +				     const u32 msg_pid,
> +				     const u32 msg_seq)
> +{

unneeded const on pass by value

> +static inline void netlbl_secattr_free(struct netlbl_lsm_secattr *secattr)
> +{
> +	BUG_ON(secattr == NULL);
> +	if (secattr->set_domain)
> +		kfree(secattr->domain);
> +	if (secattr->set_mls_cat)
> +		kfree(secattr->mls_cat);
> +	if (secattr->set_cache && secattr->cache.free)
> +		secattr->cache.free(secattr->cache.data);

The 'if' statements are not needed. You can just kfree them.

> +static inline void netlbl_secattr_destroy(struct netlbl_lsm_secattr
> *secattr) +{
> +	BUG_ON(secattr == NULL);
> +	if (secattr->set_domain)
> +		kfree(secattr->domain);
> +	if (secattr->set_mls_cat)
> +		kfree(secattr->mls_cat);
> +	if (secattr->set_cache && secattr->cache.free)
> +		secattr->cache.free(secattr->cache.data);

same

-Steve