Re: [PATCH RFC 04/20] libxc/xc_sr_save.c: add WRITE_TRIVIAL_RECORD_FN()

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Joshua Otto <jtotto@uwaterloo.ca>, xen-devel@lists.xenproject.org
Cc: ian.jackson@eu.citrix.com, hjarmstr@uwaterloo.ca,
	wei.liu2@citrix.com, czylin@uwaterloo.ca, imhy.yang@gmail.com
Subject: Re: [PATCH RFC 04/20] libxc/xc_sr_save.c: add WRITE_TRIVIAL_RECORD_FN()
Date: Tue, 28 Mar 2017 20:03:26 +0100	[thread overview]
Message-ID: <71a4f244-bc4a-977e-ab6f-07a4a0c1163f@citrix.com> (raw)
In-Reply-To: <1490605592-12189-5-git-send-email-jtotto@uwaterloo.ca>

On 27/03/17 10:06, Joshua Otto wrote:
> Writing the libxc save stream requires writing a few 'trivial' records,
> consisting only of a header with a particular type.  As a readability
> aid, it's nice to have obviously-named functions that write these sorts
> of records into the stream - for example, the first such function was
> write_end_record(), which reads much more pleasantly at its call-site
> than write_generic_record(REC_TYPE_END) would.  However, it's tedious
> and error-prone to copy-paste the generic body of such a function for
> each new trivial record type.
>
> Add a helper macro that takes a name base and a record type and declares
> the corresponding trivial record write function.  Use this to re-define
> the two existing trivial record functions, write_end_record() and
> write_checkpoint_record().
>
> No functional change.
>
> Signed-off-by: Joshua Otto <jtotto@uwaterloo.ca>

-1.

This hides the functions from tools like cscope, and makes the code
harder to read.  I also don't really buy the error prone argument.

If you do want to avoid opencoding different functions, how about

static int write_zerolength_record(uint32_t record_type)

and updating the existing callsites to be

write_zerolength_record(REC_TYPE_END); etc.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel