From patchwork Tue Apr 25 01:11:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 20129 Received: (qmail 35675 invoked by alias); 25 Apr 2017 01:11:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 35441 invoked by uid 89); 25 Apr 2017 01:11:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=Methods, Ada, continued X-HELO: mail-wr0-f178.google.com Received: from mail-wr0-f178.google.com (HELO mail-wr0-f178.google.com) (209.85.128.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Apr 2017 01:11:43 +0000 Received: by mail-wr0-f178.google.com with SMTP id l50so26671119wrc.3 for ; Mon, 24 Apr 2017 18:11:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=ebrN+Kqrcc3vZss7EDiuHxy4P0vzT71C2loJS+QQv2U=; b=JZ1RLWs4zy/+oJYk6SlGFP8h381GMM8OXGiqzM5TySKWOEYm5mbtJQ2ewjotxIAPuB NHJzYeMTxgSdeiI1zIVlb47hQXautvqzze6oph/j2jTJfTuYRgZZe8Avu/QINvXY5Myo tAsrq94YqSpwtnZ6dVTXZEnlryrCa3xJX2Gf8nljC685wC52osahJ7HNagR+6U9RmznG csWhWL9baeArXYXYi/RmfoI2h9PKzm8qig8PXt4bj2DmaalQ+iuHfrXXqvRc15yt6L/O b1NLSvg4ZzN2IAPEwaJn9TKYIcgedeJ5wOVs8W35Tr+LqkSRtcf9PV8E2aELYokoFpfI pV1Q== X-Gm-Message-State: AN3rC/4pda73zCML7pYIc1FbrsdYU2aCBmV7fyUMNoalrjSLDuFZh0WW GTEc3mCmhJhbCRHw X-Received: by 10.223.151.6 with SMTP id r6mr7838015wrb.189.1493082703055; Mon, 24 Apr 2017 18:11:43 -0700 (PDT) Received: from [192.168.0.102] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id b188sm1637188wmh.6.2017.04.24.18.11.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Apr 2017 18:11:42 -0700 (PDT) Subject: Re: [PATCH 5/5] Don't memset non-POD types: struct breakpoint To: Simon Marchi References: <1492050475-9238-1-git-send-email-palves@redhat.com> <1492050475-9238-6-git-send-email-palves@redhat.com> <97e100ef6f6427505651a2c2a90b675a@polymtl.ca> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <4a9da32e-f3ed-5ee2-c61c-9ee3519e4295@redhat.com> Date: Tue, 25 Apr 2017 02:11:41 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <97e100ef6f6427505651a2c2a90b675a@polymtl.ca> On 04/20/2017 04:58 AM, Simon Marchi wrote: >> >> b->ops = ops; >> @@ -7453,17 +7451,9 @@ init_raw_breakpoint_without_location (struct >> breakpoint *b, >> b->gdbarch = gdbarch; >> b->language = current_language->la_language; >> b->input_radix = input_radix; >> - b->thread = -1; >> b->enable_state = bp_enabled; > > Is there a reason you don't assign bp_enabled in-class directly? > Yes -- the reason is that I missed it. :-) > I think you can remove the assignment to frame_id, since it's done > in-class. Indeed. Removed. >> - struct breakpoint *next; >> + breakpoint *next = NULL; >> /* Type of breakpoint. */ >> - enum bptype type; >> + bptype type {bp_none}; > > For consistency, I think it would be nice to use = when possible > > bptype type = bp_none; Agreed. I started out by writing "{}" in all the enums, to get zero initialization without bothering to look up what the "zero" enum value was, and then when I found it, I didn't think to use =... Below's what I pushed. Thanks, Pedro Alves From 16c4d54a71d8052988ed9c8005a03a7f934245f4 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 25 Apr 2017 01:27:42 +0100 Subject: [PATCH] Don't memset non-POD types: struct breakpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eh, struct breakpoint was made non-POD just today, with commit d28cd78ad820e3 ("Change breakpoint event locations to event_location_up"). :-) src/gdb/breakpoint.c: In function ‘void init_raw_breakpoint_without_location(breakpoint*, gdbarch*, bptype, const breakpoint_ops*)’: src/gdb/breakpoint.c:7447:28: error: use of deleted function ‘void* memset(T*, int, size_t) [with T = breakpoint; = void; size_t = long unsigned int]’ memset (b, 0, sizeof (*b)); ^ In file included from src/gdb/common/common-defs.h:85:0, from src/gdb/defs.h:28, from src/gdb/breakpoint.c:20: src/gdb/common/poison.h:56:7: note: declared here void *memset (T *s, int c, size_t n) = delete; ^ gdb/ChangeLog: 2017-04-25 Pedro Alves * breakpoint.h (struct breakpoint): In-class initialize all fields. Make boolean fields "bool". * breakpoint.c (init_raw_breakpoint_without_location): Remove memset call and initializations no longer necessary. --- gdb/ChangeLog | 7 +++++++ gdb/breakpoint.c | 12 ------------ gdb/breakpoint.h | 60 ++++++++++++++++++++++++++++---------------------------- 3 files changed, 37 insertions(+), 42 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1b983b9..dc321a2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,12 @@ 2017-04-25 Pedro Alves + * breakpoint.h (struct breakpoint): In-class initialize all + fields. Make boolean fields "bool". + * breakpoint.c (init_raw_breakpoint_without_location): Remove + memset call and initializations no longer necessary. + +2017-04-25 Pedro Alves + * btrace.c (pt_btrace_insn_flags): Change parameter type to reference. (pt_btrace_insn): New function. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4db32ab..67d5988 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7428,8 +7428,6 @@ init_raw_breakpoint_without_location (struct breakpoint *b, enum bptype bptype, const struct breakpoint_ops *ops) { - memset (b, 0, sizeof (*b)); - gdb_assert (ops != NULL); b->ops = ops; @@ -7437,17 +7435,7 @@ init_raw_breakpoint_without_location (struct breakpoint *b, b->gdbarch = gdbarch; b->language = current_language->la_language; b->input_radix = input_radix; - b->thread = -1; - b->enable_state = bp_enabled; - b->next = 0; - b->silent = 0; - b->ignore_count = 0; - b->commands = NULL; - b->frame_id = null_frame_id; - b->condition_not_parsed = 0; - b->py_bp_object = NULL; b->related_breakpoint = b; - b->location = NULL; } /* Helper to set_raw_breakpoint below. Creates a breakpoint diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 18b284f..26b0aa5 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -681,45 +681,45 @@ extern int target_exact_watchpoints; struct breakpoint { /* Methods associated with this breakpoint. */ - const struct breakpoint_ops *ops; + const breakpoint_ops *ops = NULL; - struct breakpoint *next; + breakpoint *next = NULL; /* Type of breakpoint. */ - enum bptype type; + bptype type = bp_none; /* Zero means disabled; remember the info but don't break here. */ - enum enable_state enable_state; + enum enable_state enable_state = bp_enabled; /* What to do with this breakpoint after we hit it. */ - enum bpdisp disposition; + bpdisp disposition = disp_del; /* Number assigned to distinguish breakpoints. */ - int number; + int number = 0; /* Location(s) associated with this high-level breakpoint. */ - struct bp_location *loc; + bp_location *loc = NULL; - /* Non-zero means a silent breakpoint (don't print frame info if we - stop here). */ - unsigned char silent; - /* Non-zero means display ADDR_STRING to the user verbatim. */ - unsigned char display_canonical; + /* True means a silent breakpoint (don't print frame info if we stop + here). */ + bool silent = false; + /* True means display ADDR_STRING to the user verbatim. */ + bool display_canonical = false; /* Number of stops at this breakpoint that should be continued automatically before really stopping. */ - int ignore_count; + int ignore_count = 0; /* Number of stops at this breakpoint before it will be disabled. */ - int enable_count; + int enable_count = 0; /* Chain of command lines to execute when this breakpoint is hit. */ - struct counted_command_line *commands; + counted_command_line *commands = NULL; /* Stack depth (address of frame). If nonzero, break only if fp equals this. */ - struct frame_id frame_id; + struct frame_id frame_id = null_frame_id; /* The program space used to set the breakpoint. This is only set for breakpoints which are specific to a program space; for non-thread-specific ordinary breakpoints this is NULL. */ - struct program_space *pspace; + program_space *pspace = NULL; /* Location we used to set the breakpoint. */ event_location_up location; @@ -727,60 +727,60 @@ struct breakpoint /* The filter that should be passed to decode_line_full when re-setting this breakpoint. This may be NULL, but otherwise is allocated with xmalloc. */ - char *filter; + char *filter = NULL; /* For a ranged breakpoint, the location we used to find the end of the range. */ event_location_up location_range_end; /* Architecture we used to set the breakpoint. */ - struct gdbarch *gdbarch; + struct gdbarch *gdbarch = NULL; /* Language we used to set the breakpoint. */ - enum language language; + enum language language = language_unknown; /* Input radix we used to set the breakpoint. */ - int input_radix; + int input_radix = 0; /* String form of the breakpoint condition (malloc'd), or NULL if there is no condition. */ - char *cond_string; + char *cond_string = NULL; /* String form of extra parameters, or NULL if there are none. Malloc'd. */ - char *extra_string; + char *extra_string = NULL; /* Holds the address of the related watchpoint_scope breakpoint when using watchpoints on local variables (might the concept of a related breakpoint be useful elsewhere, if not just call it the watchpoint_scope breakpoint or something like that. FIXME). */ - struct breakpoint *related_breakpoint; + breakpoint *related_breakpoint = NULL; /* Thread number for thread-specific breakpoint, or -1 if don't care. */ - int thread; + int thread = -1; /* Ada task number for task-specific breakpoint, or 0 if don't care. */ - int task; + int task = 0; /* Count of the number of times this breakpoint was taken, dumped with the info, but not used for anything else. Useful for seeing how many times you hit a break prior to the program aborting, so you can back up to just before the abort. */ - int hit_count; + int hit_count = 0; /* Is breakpoint's condition not yet parsed because we found no location initially so had no context to parse the condition in. */ - int condition_not_parsed; + int condition_not_parsed = 0; /* With a Python scripting enabled GDB, store a reference to the Python object that has been associated with this breakpoint. This is always NULL for a GDB that is not script enabled. It can sometimes be NULL for enabled GDBs as not all breakpoint types are tracked by the scripting language API. */ - struct gdbpy_breakpoint_object *py_bp_object; + gdbpy_breakpoint_object *py_bp_object = NULL; /* Same as py_bp_object, but for Scheme. */ - struct gdbscm_breakpoint_object *scm_bp_object; + gdbscm_breakpoint_object *scm_bp_object = NULL; }; /* An instance of this type is used to represent a watchpoint. It