Re: [Xen-devel] [PATCH v5 1/3] golang/xenlight: begin Go to C type marshaling
On 1/16/20 4:50 PM, Nick Rosbrook wrote:
>> Looks good! Only one question:
>>
>>> +if not is_castable:
>>> +s += 'if err := x.{}.toC({}); err != nil
>>> {{\n'.format(goname,cname)
>>
>> Err should be defined function-wide at this point. Are you using `:=`
>> on purpose for some reason? Would it make sense to make this `=` instead?
>
> This is on purpose. IMO it's better to keep the variable scoped to the
> if block when using short statement if syntax. If the desire is to
> assign to the same `err` variable, then I would not use the short
> statement. Reason being I think it helps readability; the short
> statement syntax is handy, but it is easy to glance over and not
> realize an assignment is being made.
Fair enough. I'll check this series in once I get a chance to test it
again.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/3] golang/xenlight: begin Go to C type marshaling
> Looks good! Only one question:
>
> > +if not is_castable:
> > +s += 'if err := x.{}.toC({}); err != nil
> > {{\n'.format(goname,cname)
>
> Err should be defined function-wide at this point. Are you using `:=`
> on purpose for some reason? Would it make sense to make this `=` instead?
This is on purpose. IMO it's better to keep the variable scoped to the
if block when using short statement if syntax. If the desire is to
assign to the same `err` variable, then I would not use the short
statement. Reason being I think it helps readability; the short
statement syntax is handy, but it is easy to glance over and not
realize an assignment is being made.
Thanks,
-NR
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/3] golang/xenlight: begin Go to C type marshaling
On 1/4/20 9:00 PM, Nick Rosbrook wrote:
> Implement conversions for basic types such as strings and integer
> types in toC functions.
>
> Modify function signatures of toC implementations for builtin
> types to be consistent with the signature of the generated toC
> functions.
>
> Signed-off-by: Nick Rosbrook
> ---
> Changes in v5:
> - Define xenlight_golang_convert_to_C so that field conversion code
> can be easily re-used.
> - Check for err in defer'd func within toC to determine if the dispose
> function needs to be called.
> - Pass a reference to the C type in toC, rather than returning a copy
> of the C variable.
> - Update the existing toC functions for builtin types to be consistent
> with the generated functions.
> - Only call CString if the Go string is non-empty.
Looks good! Only one question:
> +if not is_castable:
> +s += 'if err := x.{}.toC({}); err != nil
> {{\n'.format(goname,cname)
Err should be defined function-wide at this point. Are you using `:=`
on purpose for some reason? Would it make sense to make this `=` instead?
If we want to, I can change that on check-in; so either way:
Reviewed-by: George Dunlap
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
