From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLbW7-00007t-SZ for qemu-devel@nongnu.org; Tue, 09 Oct 2012 11:14:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLbW3-0004Om-Ha for qemu-devel@nongnu.org; Tue, 09 Oct 2012 11:14:43 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:65435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLbW3-0004Od-CZ for qemu-devel@nongnu.org; Tue, 09 Oct 2012 11:14:39 -0400 Received: by mail-ob0-f173.google.com with SMTP id wc18so4597072obb.4 for ; Tue, 09 Oct 2012 08:14:38 -0700 (PDT) From: Anthony Liguori In-Reply-To: <506D9D5E.70303@redhat.com> References: <1349280245-16341-1-git-send-email-avi@redhat.com> <1349280245-16341-9-git-send-email-avi@redhat.com> <87pq4yib8y.fsf@codemonkey.ws> <506D9D5E.70303@redhat.com> Date: Tue, 09 Oct 2012 10:14:32 -0500 Message-ID: <87391nn0dj.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC v1 08/22] memory: provide defaults for MemoryListener operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Blue Swirl , Paolo Bonzini , "Michael S. Tsirkin" , qemu-devel@nongnu.org, liu ping fan Avi Kivity writes: > On 10/04/2012 04:05 PM, Anthony Liguori wrote: >> Avi Kivity writes: >> >>> Many listeners don't need to respond to all MemoryListener callbacks; >>> provide suitable defaults instead. >>> > >>> +#define MEMORY_LISTENER_DEFAULT_OPS \ >>> + .begin = memory_listener_default_global, \ >>> + .commit = memory_listener_default_global, \ >>> + .region_add = memory_listener_default_section, \ >>> + .region_del = memory_listener_default_section, \ >>> + .region_nop = memory_listener_default_section, \ >>> + .log_start = memory_listener_default_section, \ >>> + .log_stop = memory_listener_default_section, \ >>> + .log_sync = memory_listener_default_section, \ >>> + .log_global_start = memory_listener_default_global, \ >>> + .log_global_stop = memory_listener_default_global, \ >>> + .eventfd_add = memory_listener_default_eventfd, \ >>> + .eventfd_del = memory_listener_default_eventfd \ >>> + >>> +void memory_listener_default_global(MemoryListener *listener); >>> +void memory_listener_default_section(MemoryListener *listener, >>> + MemoryRegionSection *section); >>> +void memory_listener_default_eventfd(MemoryListener *listener, >>> + MemoryRegionSection *section, >>> + bool match_data, uint64_t data, EventNotifier *e); >>> + >>> /** >>> * memory_region_init: Initialize a memory region >>> * >> >> I think it'd be nicer to check for NULL when invoking the functions in >> the memory core. >> >> Then you avoid the exported stub functions entirely. > > Yes, that's the common style, but I happen not to like the extra check, > both from a performance point of view (doesn't apply here of course) and > from a readability point of view. The trouble with your approach is that it introduced a subtle behavior based on ordering. IOW: MemoryListenerOps foo = { MEMORY_LISTENER_DEFAULT_OPS, .log_sync = ..., }; vs. MemoryListenerOps foo = { .log_sync = ..., MEMORY_LISTENER_DEFAULT_OPS, }; Both compile fine but have potentially difficult to debug differences. Relying on zero-initialization eliminates the possibility of this problem. Regards, Anthony Liguori > > -- > error compiling committee.c: too many arguments to function