Re: [PATCH 11/12] OvmfPkg/SmmControl2Dxe: save fw_cfg boot script with QemuFwCfgS3Lib

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-02-22 17:48:13, Laszlo Ersek wrote:
> +  //
> +  ScratchBuffer->Features = mSmiFeatures;
> +  Status = QemuFwCfgS3WriteBytes (mRequestedFeaturesItem,

I noticed a fair amount of function calls where the first args are
appearing on the first line, and then the rest appear on subsequent
lines indented two spaces.

> +             sizeof ScratchBuffer->Features);
> +  if (RETURN_ERROR (Status)) {
> +    goto FatalError;
> +  }
> +

I can't say that the coding style prohibits this, but it does seem odd
looking. I would expect to see:

[1]

  Status = QemuFwCfgS3WriteBytes (
             mRequestedFeaturesItem,
             sizeof ScratchBuffer->Features
             );

The coding style examples look like this:

  Status = QemuFwCfgS3WriteBytes (
             mRequestedFeaturesItem,
             sizeof ScratchBuffer->Features
           );

But this looks less like what I'm used to actually seeing in EDK II.
(Look at DXE Core, for example. It looks like [1].)

I personally think this is okay to save a line, but it doesn't seem to
follow the coding style doc:

  Status = QemuFwCfgS3WriteBytes (
             mRequestedFeaturesItem,
             sizeof ScratchBuffer->Features);

Now, we all know EDK II has a style of it's own, but outide EDK II, I
might expect something like:

  Status = QemuFwCfgS3WriteBytes(mRequestedFeaturesItem,
                                 sizeof ScratchBuffer->Features);

Which once again shows arguements on subsequent lines are lined up.
Based on that, I think [1] is the best style for EDK II code.

-Jordan