[PATCH openbmc v2] Generate code from conf files from IPMI packages

[Patrick Williams]

On Fri, Jul 01, 2016 at 09:30:37AM -0500, OpenBMC Patches wrote:
> From: tomjose <tomjoseph at in.ibm.com>
> 
> IPMI whitelisted commands for each package is mentioned in the
> respective conf file. The conf files are merged and code is
> generated to be consumed by ipmi daemon.

> --- a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb
> +++ b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid.bb
> @@ -17,7 +17,10 @@ TARGET_CFLAGS   += "-fpic"
>  RDEPENDS_${PN} += "clear-once"
>  RDEPENDS_${PN} += "settings"
>  RDEPENDS_${PN} += "network"
> -SRC_URI += "git://github.com/openbmc/phosphor-host-ipmid"
> +SRC_URI += "git://github.com/openbmc/phosphor-host-ipmid \
> +			file://host-ipmid-whitelist.conf \
> +			file://host-ipmid-oem-whitelist.conf \
> +			file://host-ipmid-fru-whitelist.conf"

Shouldn't the oem and fru whitelist entries be provided by their
respective packages?

> +do_configure () {
> +        merge_config.sh -m .config ${@" ".join(find_cfgs(d))}

I don't think merge_config is needed here.  The 'generate_whitelist'
appears to take a list of files already.

> +        generate_whitelist .config  > ipmiwhitelist.C
> +}
> +
> +generate_whitelist() {

I think we should move this script into the IPMI package itself.  The
file output format is tied to the IPMI code (vector<pair<uchar,
uchar>>).

> +        # Ensure some files have been passed.
> +        if [ "x$*" == "x" ]; then
> +            echo "Usage: $0 [whitelist_files+]" >&2
> +            exit -1
> +        fi
> +		|

Check for excess whitespace throughout this function.  A lot of stray
tabs.

> +        # Output header of C++ file.
> +        echo -e '#include <vector>\n'
> +		
> +        echo -e 'using netfncmd_pair = std::pair<unsigned char, unsigned char>;\n'

I think there is a header that defines this type already, no?  Should we
include it instead?

> +		
> +        echo -e 'const std::vector<netfncmd_pair> whitelist = {\n'
> +		
> +        # Output each row of whitelist vector.
> +        # Concatenate all the passed files.
> +        # Remove comments and empty lines.
> +        # Sort the list [numerically].
> +        # Remove any duplicates.
> +        # Turn "a:b" -> "{ a, b },"
> +        cat $* | sed "s/#.*//" | sed '/^$/d' | sort -n | uniq | sed "s/^/    { /" | \
> +        sed "s/:/, /" | sed "s/$/ }, /"
> +    	
> +        echo '};'
> +}

> +++ b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmid/files/host-ipmid-fru-whitelist.conf
> @@ -0,0 +1,2 @@
> +#<NetFn>:<Command>
> +0x32:0xF0

I'd prefer to also see each netfn/command documented.

    0x32:0xF0   # UserManagement : SetRootPassword

(I totally made up these netfn / command definitions.  Suppose to be an
example).

-- 
Patrick Williams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160701/406a330b/attachment.sig>