Re: [JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe
- From
Robin Rosenberg <robin.rosenberg@dewire.com>
- Date
- Jul 21, 2009, 21:51 UTC
- Message-ID
- <200907212351.21760.robin.rosenberg@dewire.com>
- In-Reply-To
- <1248207570-13880-12-git-send-email-spearce@spearce.org>
tisdag 21 juli 2009 22:19:29 skrev "Shawn O. Pearce" <spearce@spearce.org>:
Show 31 quoted lines
> The magic value "%%magic%%empty%%" is just too magic; if it ever > did appear as a value in a key string Config would have treated > it as a true value instead of as a string value. We also had to > special case conversions of it to the empty string in a string > context. Instead we create a special String object using the > empty string as a template, and use reference equality against > that to indicate the magic empty value. > > Signed-off-by: Shawn O. Pearce <spearce@spearce.org> > --- > .../src/org/spearce/jgit/lib/Config.java | 28 +++++++++---------- > 1 files changed, 13 insertions(+), 15 deletions(-) > > diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java > index 974ffea..e4528b1 100644 > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java > @@ -76,9 +76,14 @@ > private Map<String, Object> byName; > > /** > - * Magic value indicating a missing entry > + * Magic value indicating a missing entry. > + * <p> > + * This value is tested for reference equality in some contexts, so we > + * must ensure it is a special copy of the empty string. It also must > + * be treated like the empty string. > */ > - private static final String MAGIC_EMPTY_VALUE = "%%magic%%empty%%"; > + private static final String MAGIC_EMPTY_VALUE = new StringBuilder(0) > + .toString();
Can we be sure an implementation doesn't "optimize" toString() here? But an explicit new String() shouldn't be..?
-- robin
Show 53 quoted lines
> /**
> * The constructor for configuration file
> @@ -293,7 +298,7 @@ public boolean getBoolean(final String section, String subsection,
> if (n == null)
> return defaultValue;
>
> - if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n)
> + if (MAGIC_EMPTY_VALUE == n || "yes".equalsIgnoreCase(n)
> || "true".equalsIgnoreCase(n) || "1".equals(n)
> || "on".equalsIgnoreCase(n)) {
> return true;
> @@ -321,11 +326,7 @@ public boolean getBoolean(final String section, String subsection,
> */
> public String getString(final String section, String subsection,
> final String name) {
> - String val = getRawString(section, subsection, name);
> - if (MAGIC_EMPTY_VALUE.equals(val)) {
> - return "";
> - }
> - return val;
> + return getRawString(section, subsection, name);
> }
>
> /**
> @@ -345,16 +346,13 @@ public String getString(final String section, String subsection,
> if (o instanceof List) {
> final List lst = (List) o;
> final String[] r = new String[lst.size()];
> - for (int i = 0; i < r.length; i++) {
> - final String val = ((Entry) lst.get(i)).value;
> - r[i] = MAGIC_EMPTY_VALUE.equals(val) ? "" : val;
> - }
> + for (int i = 0; i < r.length; i++)
> + r[i] = ((Entry) lst.get(i)).value;
> return r;
> }
>
> if (o instanceof Entry) {
> - final String val = ((Entry) o).value;
> - return new String[] { MAGIC_EMPTY_VALUE.equals(val) ? "" : val };
> + return new String[] { ((Entry) o).value };
> }
>
> if (baseConfig != null)
> @@ -700,7 +698,7 @@ protected void printConfig(final PrintWriter r) {
> }
> r.print(e.name);
> if (e.value != null) {
> - if (!MAGIC_EMPTY_VALUE.equals(e.value)) {
> + if (MAGIC_EMPTY_VALUE != e.value) {
> r.print(" = ");
> r.print(escapeValue(e.value));
> }