[Spice-devel] [PATCH spice-common v2] codegen: Add some comments

[Lukáš Hrázký]

Sorry for being pedantic :)

On Wed, 2018-05-09 at 10:05 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  python_modules/ptypes.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Changes since v1:
> - grammar and better comments
> 
> diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> index 209c00e..cfdd17e 100644
> --- a/python_modules/ptypes.py
> +++ b/python_modules/ptypes.py
> @@ -83,6 +83,8 @@ valid_attributes=set([
>      'nomarshal',
>      # ??? not used by python code
>      'zero_terminated',
> +    # force generating marshaller code, apply to pointers which by
> +    # default are not marshalled (submarshallers are generated)

Didn't catch this first time, is "apply" supposed to read "applies"?

>      'marshall',
>      # this pointer member cannot be null
>      'nonnull',
> @@ -97,6 +99,8 @@ valid_attributes=set([
>      # the argument specifies the preprocessor define to check
>      'ifdef',
>      # write this member as zero on network
> +    # marshalling data a zero field is written to network
> +    # demarshalling data field is read from network and discarded

Not a great english, perhaps:
# when marshalling, a zero field is written to the network
# when demarshalling, the field is read from the network and discarded

Though I'd leave the "written/read to/from the network", as it's not
technically part of the (de)marshalling.

>      'zero',
>      # specify minor version required for these members
>      'minor',
> @@ -124,7 +128,7 @@ attributes_with_arguments=set([
>  def fix_attributes(attribute_list):
>      attrs = {}
>      for attr in attribute_list:
> -        name = attr[0][1:]
> +        name = attr[0][1:] # [1:] strips leading '@' from name

# [1:] strips the leading '@'

(definite particles are missing in front of both "leading '@'" and
"name")

:)

Lukas

>          lst = attr[1:]
>          if not name in valid_attributes:
>              raise Exception("Attribute %s not recognized" % name)