From patchwork Tue Nov 15 23:02:18 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 17496 Received: (qmail 7604 invoked by alias); 15 Nov 2016 23:02:43 -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 7594 invoked by uid 89); 15 Nov 2016 23:02:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS, URIBL_RED autolearn=no version=3.3.2 spammy=$subdir, sk:$defaul, U*lgustavo, $srcdir X-HELO: mail-wm0-f52.google.com Received: from mail-wm0-f52.google.com (HELO mail-wm0-f52.google.com) (74.125.82.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Nov 2016 23:02:32 +0000 Received: by mail-wm0-f52.google.com with SMTP id g23so200244248wme.1 for ; Tue, 15 Nov 2016 15:02:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=2bMhZvajBGpC2xmRtGzBdswLW7WJabtAp35T3p2ovys=; b=WWBMJsotkXG5KGOmWOVdoUalCb/clBLVnIzdES+PEPKFp7VJPAoQKE5ioAE6GDmyhM /toZTmMRh+NPjict7GP2WZqImvf3dCt+xcIMe0Bq6EdfN8zfyn8yHi3DCFzgN6JBgRZA lZkx47sWau+FGI9vEApOrf7w/eDAOKVVXQs+XobuuhW6/G4BHgFhXkeRVOfBJ2UjE/HV iuMHhW/rX/15rtCrsXsdihjcwPGJzt1kmAPQlzZ9chGYNjqTv/Oj8NsMmTDUi+am0d2y 0JYBkXx5PxkhLwP0ZKC3Rsfr20bQgnlpT0dq0trdylLNrz0bWk8Iwcp5IIQnqdc9jtlF RuUw== X-Gm-Message-State: ABUngvevIICfKOL5fphT4lWjTTOFjl3Ue5qWNOK4B+JKQJe04eit4Ww2ZNaSaIsg3wKLbQ== X-Received: by 10.28.146.203 with SMTP id u194mr6067007wmd.79.1479250949573; Tue, 15 Nov 2016 15:02:29 -0800 (PST) Received: from localhost (host86-135-139-198.range86-135.btcentralplus.com. [86.135.139.198]) by smtp.gmail.com with ESMTPSA id d8sm6629849wmi.21.2016.11.15.15.02.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Nov 2016 15:02:28 -0800 (PST) Date: Tue, 15 Nov 2016 23:02:18 +0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Luis Machado Subject: Re: [PATCH 1/2] gdb: Restore a settings previous value on error during change Message-ID: <20161115230218.GC5975@embecosm.com> References: <1478888325-32039-1-git-send-email-andrew.burgess@embecosm.com> <1478888325-32039-2-git-send-email-andrew.burgess@embecosm.com> <8ef10d71-c977-d0ee-c6c5-f4b1aac3f55a@codesourcery.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8ef10d71-c977-d0ee-c6c5-f4b1aac3f55a@codesourcery.com> X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.6.1 (2016-04-27) X-IsSubscribed: yes * Luis Machado [2016-11-11 18:20:24 -0600]: > On 11/11/2016 12:18 PM, Andrew Burgess wrote: > > When changing the value of a setting in do_set_command, there are three > > phases: > > > > 1. The old value is destroyed, and the new value is placed into the > > settings variable. > > > > 2. The set-hook, the 'func' field of the setting is called. > > > > 3. A change notification is issued. > > > > There are two problems that I try to address in this commit. > > > > 1. If the set-hook does not like the new value then either the setting > > is restored to a default value, or we have to have a complex system > > within the set-hook to remember the previous value and restore it > > manually when an invalid change is made. > > > > 2. If the set-hook throws an error then the change notification is > > skipped, even though the setting might still have changed (say, back to > > some default value). > > > > The solution I propose here is to delay destroying the old value of the > > setting until after the set-hook has been called. If the set-hook runs > > successfully then the old value will be destroyed when do_set_command > > returns, the new value will be left in place, and the change > > notification will be sent sent out exactly as before. > > > > However, if the set-hook throws an error this is now caught in > > do_set_command, the new value of the setting is removed, and the old > > value is restored. > > > > After this change, it is no longer possible in a set-hook to change a > > setting to a default value _and_ throw an error. If you throw an error > > you should assume that gdb will restore the previous value of the > > setting. If you want to change a setting in the set-hook and inform the > > user, then a warning, or just a standard message should be fine. > > > > gdb/ChangeLog: > > > > * cli/cli-setshow.c: Add exceptions.h include. > > (do_set_comment): When installing a settings new value, don't > > loose the previous value until the set-hook has been called. If > > the set-hook throws an error, restore the settings previous value. > > > > gdb/testsuite/ChangeLog: > > > > * gdb.base/dcache-line-set-error.exp: New file. > > --- > > gdb/ChangeLog | 7 +++ > > gdb/cli/cli-setshow.c | 80 ++++++++++++++++++++++-- > > gdb/testsuite/ChangeLog | 4 ++ > > gdb/testsuite/gdb.base/dcache-line-set-error.exp | 67 ++++++++++++++++++++ > > 4 files changed, 153 insertions(+), 5 deletions(-) > > create mode 100644 gdb/testsuite/gdb.base/dcache-line-set-error.exp > > > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > > index 7fc2b4a..3cb115f 100644 > > --- a/gdb/ChangeLog > > +++ b/gdb/ChangeLog > > @@ -1,3 +1,10 @@ > > +2016-11-11 Andrew Burgess > > + > > + * cli/cli-setshow.c: Add exceptions.h include. > > + (do_set_comment): When installing a settings new value, don't > > + loose the previous value until the set-hook has been called. If > > + the set-hook throws an error, restore the settings previous value. > > + > > "a setting's new ..." > "don't lose ..." > "restore the setting's..." > > > @@ -151,8 +152,20 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) > > { > > /* A flag to indicate the option is changed or not. */ > > int option_changed = 0; > > + /* Cleanups created to free the previous value of the setting. */ > > + struct cleanup *cleanups; > > + /* The previous value of the setting. Restored if the set-hook function > > + throws an error. */ > > + union > > + { > > + enum auto_boolean b; > > + int i; > > + const char *s; > > + unsigned int u; > > + } prev_value; > > I wonder if we can share and potentially simplify the storage mechanism for > these settings with what struct cmd_list_element already has? As opposed to > declaring something new just for this particular function? I'm not sure how we'd do that. The 'struct cmd_list_element' holds a 'void *' pointer to a variable of _some type_ the interpretation of that 'void *' depends on the 'var_type' field within the 'struct cmd_list_element'. But we never actually hold the _value_ of the setting in the 'struct cmd_list_element'. So I think that we will always end up introducing _some_ new structure to hold the previous value. I'm open to suggestions though, just because I can't see the solution doesn't mean it's not there... > Since we're already touching this function, it may be worthwhile to get it > cleaned up a bit if that's not too much effort. I can do some clean up if you'd like, I can even make it part of this patch series, though obviously it'll be a new patch in this series. But you'll need to give some more specific indication of what you'd like to see changed.... > > I also see a lot of repetition trying to save the old value. If we could get > it saved at the function's entry, that would save many lines of code, but i > think this is a nice-to-have and not required for this patch. I'm not sure I'd agree with "many", but in the new version I've tried to reduce the number of new lines added. As I discuss above, given that the 'struct cmd_list_element' only has a 'void *' pointer, backing up the previous value has to be done on a case by case basis. Again, though, if you have a specific idea in mind that I'm not seeing, feel free to make a suggestion, I'm happy to change things. > > > > > gdb_assert (c->type == set_cmd); > > + cleanups = make_cleanup (null_cleanup, NULL); > > > > switch (c->var_type) > > { > > @@ -200,7 +213,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) > > if (*(char **) c->var == NULL > > || strcmp (*(char **) c->var, newobj) != 0) > > { > > - xfree (*(char **) c->var); > > + prev_value.s = *(const char **) c->var; > > + make_cleanup (xfree, *(char **) c->var); > > *(char **) c->var = newobj; > > For example, this type of procedure of saving the old value repeats itself > over and... > > > > option_changed = 1; > > @@ -215,7 +229,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) > > > > if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0) > > { > > - xfree (*(char **) c->var); > > + prev_value.s = *(const char **) c->var; > > + make_cleanup (xfree, *(char **) c->var); > > *(char **) c->var = xstrdup (arg); > > > > ... over and ... > > option_changed = 1; > > @@ -248,7 +263,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) > > if (*(char **) c->var == NULL > > || strcmp (*(char **) c->var, val) != 0) > > { > > - xfree (*(char **) c->var); > > + prev_value.s = *(const char **) c->var; > > + make_cleanup (xfree, *(char **) c->var); > > *(char **) c->var = val; > > > > ... over again. > > > @@ -452,7 +474,51 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) > > default: > > error (_("gdb internal error: bad var_type in do_setshow_command")); > > } > > - c->func (c, NULL, from_tty); > > + > > + TRY > > + { > > + c->func (c, NULL, from_tty); > > + } > > + CATCH (ex, RETURN_MASK_ERROR) > > + { > > + /* The only cleanups in place free the previous value, which we're > > + about to restore. */ > > Maybe change this comment a bit to be more to the point? > > "Discard the cleanups since we failed to set the new value." > > > + discard_cleanups (cleanups); > > + /* Now restore the previous value, and free the new value. */ > > + if (option_changed) > > + switch (c->var_type) > > + { > > + case var_string: > > + case var_string_noescape: > > + case var_filename: > > + case var_optional_filename: > > + xfree (*(char **) c->var); > > + *(char **) c->var = (char *) prev_value.s; > > + break; > > + > > + case var_boolean: > > + case var_integer: > > + case var_zinteger: > > + case var_zuinteger_unlimited: > > + *(int *) c->var = prev_value.i; > > + break; > > + > > + case var_auto_boolean: > > + *(enum auto_boolean *) c->var = prev_value.b; > > + break; > > + > > + case var_uinteger: > > + case var_zuinteger: > > + *(unsigned int *) c->var = prev_value.u; > > + break; > > + > > + case var_enum: > > + *(char **) c->var = (char *) prev_value.s; > > + break; > > + } > > + throw_exception (ex); > > + } > > + END_CATCH > > Can we make a cleanup that just restores some old value regardless of its > type so we can call it once without having to check what kind of variable > we're dealing with (since we would have checked that before getting here > already and before putting such cleanup in place?). > > It sounds like we could use a couple cleanups? One to free the old value due > to having a new valid value in place and another to restore the old value in > case the new value is invalid? > > Not sure how much work that adds, but sounds like it would make this code a > bit more clean? I've folded the whole restore or clean-up the previous value into a single cleanup function. I'm not 100% convinced that the new code is better than the previous, but I don't think it's any worse. Let me know what you think of the new version. Thanks, Andrew --- gdb: Restore a settings previous value on error during change When changing the value of a setting in do_set_command, there are three phases: 1. The old value is destroyed, and the new value is placed into the settings variable. 2. The set-hook, the 'func' field of the setting is called. 3. A change notification is issued. There are two problems that I try to address in this commit. 1. If the set-hook does not like the new value then either the setting is restored to a default value, or we have to have a complex system within the set-hook to remember the previous value and restore it manually when an invalid change is made. 2. If the set-hook throws an error then the change notification is skipped, even though the setting might still have changed (say, back to some default value). The solution I propose here is to delay destroying the old value of the setting until after the set-hook has been called. If the set-hook runs successfully then the old value will be destroyed when do_set_command returns, the new value will be left in place, and the change notification will be sent sent out exactly as before. However, if the set-hook throws an error this is now caught in do_set_command, the new value of the setting is removed, and the old value is restored. After this change, it is no longer possible in a set-hook to change a setting to a default value _and_ throw an error. If you throw an error you should assume that gdb will restore the previous value of the setting. If you want to change a setting in the set-hook and inform the user, then a warning, or just a standard message should be fine. gdb/ChangeLog: * cli/cli-setshow.c: Add exceptions.h include. (struct set_cmd_cleanup_data): New definition. (set_cmd_cleanup_or_revert): New function. (do_set_comment): When installing a setting's new value, don't lose the previous value until the set-hook has been called. Use a cleanup to either delete or restore the previous value. gdb/testsuite/ChangeLog: * gdb.base/dcache-line-set-error.exp: New file. --- gdb/ChangeLog | 9 ++ gdb/cli/cli-setshow.c | 142 ++++++++++++++++++++++- gdb/testsuite/ChangeLog | 4 + gdb/testsuite/gdb.base/dcache-line-set-error.exp | 67 +++++++++++ 4 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/dcache-line-set-error.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7fc2b4a..bbfefe8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2016-11-11 Andrew Burgess + + * cli/cli-setshow.c: Add exceptions.h include. + (struct set_cmd_cleanup_data): New definition. + (set_cmd_cleanup_or_revert): New function. + (do_set_comment): When installing a setting's new value, don't + lose the previous value until the set-hook has been called. Use a + cleanup to either delete or restore the previous value. + 2016-11-11 Yao Qi * cp-valprint.c (cp_print_value): Remove local base_valaddr. diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index d2ec1df..b5fd75d 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -21,6 +21,7 @@ #include #include "arch-utils.h" #include "observer.h" +#include "exceptions.h" #include "ui-out.h" @@ -140,6 +141,100 @@ is_unlimited_literal (const char *arg) && (isspace (arg[len]) || arg[len] == '\0')); } +/* Used within DO_SET_COMMAND and SET_CMD_CLEANUP_OR_REVERT. Holds + information required to either cleanup the previous value of a setting, + or to restore the previous value. */ + +struct set_cmd_cleanup_data +{ + /* The setting we're working on. */ + struct cmd_list_element *c; + + /* Should we revert the setting to the previous value? If not then we + should clean up the previous value. */ + int revert_p; + + /* The previous value. Which field is used depends on c->var_type. */ + union + { + enum auto_boolean b; + int i; + const char *s; + unsigned int u; + } prev_value; +}; + +/* Cleanup function called from DO_SET_COMMAND. Either cleans up the + previous value of a setting, freeing any associated memory, or, + restores the previous value, freeing the new value of a setting. */ + +static void +set_cmd_cleanup_or_revert (void *arg) +{ + struct set_cmd_cleanup_data *set_cmd_cleanup_data + = (struct set_cmd_cleanup_data *) arg; + + if (set_cmd_cleanup_data->revert_p) + { + switch (set_cmd_cleanup_data->c->var_type) + { + case var_string: + case var_string_noescape: + case var_filename: + case var_optional_filename: + xfree (*(char **) set_cmd_cleanup_data->c->var); + *(char **) set_cmd_cleanup_data->c->var + = (char *) set_cmd_cleanup_data->prev_value.s; + break; + + case var_boolean: + case var_integer: + case var_zinteger: + case var_zuinteger_unlimited: + *(int *) set_cmd_cleanup_data->c->var + = set_cmd_cleanup_data->prev_value.i; + break; + + case var_auto_boolean: + *(enum auto_boolean *) set_cmd_cleanup_data->c->var + = set_cmd_cleanup_data->prev_value.b; + break; + + case var_uinteger: + case var_zuinteger: + *(unsigned int *) set_cmd_cleanup_data->c->var + = set_cmd_cleanup_data->prev_value.u; + break; + + case var_enum: + *(char **) set_cmd_cleanup_data->c->var + = (char *) set_cmd_cleanup_data->prev_value.s; + break; + } + } + else + { + switch (set_cmd_cleanup_data->c->var_type) + { + case var_string: + case var_string_noescape: + case var_filename: + case var_optional_filename: + xfree ((char *) set_cmd_cleanup_data->prev_value.s); + break; + + case var_boolean: + case var_integer: + case var_zinteger: + case var_zuinteger_unlimited: + case var_auto_boolean: + case var_uinteger: + case var_zuinteger: + case var_enum: + break; + } + } +} /* Do a "set" command. ARG is NULL if no argument, or the text of the argument, and FROM_TTY is nonzero if this command is @@ -151,8 +246,19 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { /* A flag to indicate the option is changed or not. */ int option_changed = 0; + /* Cleanups created to free the previous value of the setting. */ + struct cleanup *cleanups; + /* Data used to either clean up the previous value of the setting, or + restore the previous value, after we have called c->func. */ + struct set_cmd_cleanup_data set_cmd_cleanup_data; + set_cmd_cleanup_data.c = c; + set_cmd_cleanup_data.revert_p = 0; + memset (&set_cmd_cleanup_data.prev_value, 0, + sizeof (set_cmd_cleanup_data.prev_value)); gdb_assert (c->type == set_cmd); + cleanups = make_cleanup (set_cmd_cleanup_or_revert, + &set_cmd_cleanup_data); switch (c->var_type) { @@ -200,7 +306,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(char **) c->var == NULL || strcmp (*(char **) c->var, newobj) != 0) { - xfree (*(char **) c->var); + set_cmd_cleanup_data.prev_value.s = *(const char **) c->var; *(char **) c->var = newobj; option_changed = 1; @@ -215,7 +321,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0) { - xfree (*(char **) c->var); + set_cmd_cleanup_data.prev_value.s = *(const char **) c->var; *(char **) c->var = xstrdup (arg); option_changed = 1; @@ -248,7 +354,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(char **) c->var == NULL || strcmp (*(char **) c->var, val) != 0) { - xfree (*(char **) c->var); + set_cmd_cleanup_data.prev_value.s = *(const char **) c->var; *(char **) c->var = val; option_changed = 1; @@ -265,6 +371,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) error (_("\"on\" or \"off\" expected.")); if (val != *(int *) c->var) { + set_cmd_cleanup_data.prev_value.i = *(int *) c->var; *(int *) c->var = val; option_changed = 1; @@ -277,6 +384,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(enum auto_boolean *) c->var != val) { + set_cmd_cleanup_data.prev_value.b = *(enum auto_boolean *) c->var; *(enum auto_boolean *) c->var = val; option_changed = 1; @@ -313,6 +421,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(unsigned int *) c->var != val) { + set_cmd_cleanup_data.prev_value.u = *(unsigned int *) c->var; *(unsigned int *) c->var = val; option_changed = 1; @@ -349,12 +458,13 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(int *) c->var != val) { + set_cmd_cleanup_data.prev_value.i = *(int *) c->var; *(int *) c->var = val; option_changed = 1; } - break; } + break; case var_enum: { int i; @@ -419,6 +529,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(const char **) c->var != match) { + set_cmd_cleanup_data.prev_value.s = *(const char **) c->var; *(const char **) c->var = match; option_changed = 1; @@ -444,6 +555,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*(int *) c->var != val) { + set_cmd_cleanup_data.prev_value.i = *(int *) c->var; *(int *) c->var = val; option_changed = 1; } @@ -452,7 +564,24 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) default: error (_("gdb internal error: bad var_type in do_setshow_command")); } - c->func (c, NULL, from_tty); + + TRY + { + c->func (c, NULL, from_tty); + } + CATCH (ex, RETURN_MASK_ERROR) + { + /* Set the REVERT_P flag based on OPTION_CHANGED. If this leaves + REVERT_P as false then the cleanup SET_CMD_CLEANUP_OR_REVERT will + try to cleanup the previous value, but as OPTION_CHANGED was + false, there is no previous value, and so nothing is done. If + REVERT_P is changed to TRUE here then there was a previous value, + and so the SET_CMD_CLEANUP_OR_REVERT will restore the previous + value, and cleanup the new value that we no longer need. */ + set_cmd_cleanup_data.revert_p = option_changed; + throw_exception (ex); + } + END_CATCH if (notify_command_param_changed_p (option_changed, c)) { @@ -493,6 +622,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) xfree (cmds); xfree (name); + do_cleanups (cleanups); return; } /* Traverse them in the reversed order, and copy their names into @@ -557,6 +687,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) } xfree (name); } + + do_cleanups (cleanups); } /* Do a "show" command. ARG is NULL if no argument, or the diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index b2fc137..01f737e 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2016-11-11 Andrew Burgess + + * gdb.base/dcache-line-set-error.exp: New file. + 2016-11-09 Pedro Alves * gdb.base/commands.exp (runto_or_return): New procedure. diff --git a/gdb/testsuite/gdb.base/dcache-line-set-error.exp b/gdb/testsuite/gdb.base/dcache-line-set-error.exp new file mode 100644 index 0000000..976e07e --- /dev/null +++ b/gdb/testsuite/gdb.base/dcache-line-set-error.exp @@ -0,0 +1,67 @@ +# Test error handling when seting dcache line size +# Copyright 2011-2016 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir + +# Read the current dcache line-size, check default is still 64. +set default_line_size 0 +set testname "read default value for dcache line-size" +send_gdb "show dcache line-size\n" +gdb_expect { + -re "Dcache line size is (\[0-9\]+\)\.\r\n$gdb_prompt $" { + set default_line_size $expect_out(1,string) + pass $testname + } + -re ".*$gdb_prompt $" { fail $testname } + timeout { fail "$testname (timeout)" } +} + +if { $default_line_size == 0 } then { + unsupported "dcache-line-set-error" + return +} + +# Try to change to some invalid (non power of 2 value) +set invalid_line_size [expr $default_line_size - 1] +gdb_test "set dcache line-size $invalid_line_size" \ + "Invalid dcache line size: $invalid_line_size .*" \ + "First attempt to set invalid dcache line size" + +# Check the default is still 64 +gdb_test "show dcache line-size" \ + "Dcache line size is $default_line_size\." \ + "Check that the default value is still in place" + +# Change the line size to some other valid value, and check the value +# changed. +set new_line_size [expr $default_line_size * 2] +gdb_test_no_output "set dcache line-size $new_line_size" \ + "Set dcache line size to a new value" +gdb_test "show dcache line-size" \ + "Dcache line size is $new_line_size\." \ + "Check that the new value took" + +# Try to change to some invalid (non power of 2 value) +gdb_test "set dcache line-size $invalid_line_size" \ + "Invalid dcache line size: $invalid_line_size .*" \ + "Second attempt to set invalid dcache line size" + +# Check that the new value is still in place. +gdb_test "show dcache line-size" \ + "Dcache line size is $new_line_size\." \ + "Check that the new value is still in place"