On getting a meaningful review

Stefan (metze) Metzmacher metze at samba.org
Mon Nov 4 17:00:06 MST 2013


Am 04.11.2013 22:40, schrieb Matthieu Patou:
> Hello All,
> 
> I often get reviews like that:
> https://git.samba.org/?p=metze/samba/wip.git;a=commit;h=f53d760eee507343e36e0e7803a236933f10b411
> 
> 
> I do appreciate the time spent on the review but spending an extra 30s
> for telling me what is wrong would be nice because we might not be in
> timezones that allow easy interaction (read going to bed super late like
> 1AM or 2AM).



I thought the following gen_ndr diff whould make it clear that this
change is wrong, as it changes the way we handle [enum16bit] enums,
which should always be 'uint16' on the wire. Notice that all
hunks are related to structures, which are used for custom marshalled blobs,
not really dcerpc ndr/ndr64.

I thought this patch before
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=bf52fe08d35f779dbbf959affc58ec2cf3104d96
(drsuapi.idl: drsuapi_DsGetNCChangesCompressionType is just an enum)
Whould show how to fix NDR64 enum handling, for an enum that is really
used for dcderpc ndr/ndr64 marshalling.

https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=52015f77efb4174f46f67961d06550a5344b1aa8
(spoolss.idl: use spoolss_NotifyTable as switch_type for spoolss_NotifyData)
and
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=2ef4c23d614cee387dd6ad4fea20dc72dc296d38
(spoolss.idl: make use of enums in spoolss_Field)
partly show the reverse, that we use use enum16bit for custom marshalled
values,
which are 'unsigned short' in the microsoft idl.

I hope that helps...

metze


More information about the samba-technical mailing list