Bug 580873: Remove error checking on dprintf parameters

The parser for dprintf parameters was much too simplistic and prevents real uses cases from operating. It is not necessary to remove the , between the arguments to dprintf, so don't try to split on that anymore. That also means we can't check for and error on mismatch between format specifiers and number of arguments. e.g. "===> XML_EVENT_TEXT(%s)\n", (char *)strtok(Text,"\n") should be permitted. The alternative would be to write a more complete parser for both the printf specification and the arguments. Well out of scope and unnecessary. Some inputs may now be accepted by the GUI and then when inserted with GDB fail. These errors are displayed in the GUI already anyway.
eclipse-cdt · Oct 7, 2022 · 323423f · 323423f
1 parent e08950f
commit 323423f
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 50 deletions.
diff --git a/....ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GDBDynamicPrintfPropertyPage.java b/....ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GDBDynamicPrintfPropertyPage.java
@@ -43,7 +43,9 @@
 import org.eclipse.jface.util.PropertyChangeEvent;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.IStructuredSelection;
+import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Text;
 import org.eclipse.ui.IWorkbenchPropertyPage;
 import org.eclipse.ui.model.IWorkbenchAdapter;
@@ -305,6 +307,8 @@ protected void createIgnoreCountEditor(Composite parent) {
 	protected void createPrintStringEditor(Composite parent) {
 		fPrintString = new DynamicPrintfStringFieldEditor(ICDynamicPrintf.PRINTF_STRING,
 				Messages.DynamicPrintfPropertyPage_PrintString, parent) {
+			private Label closingParenLabel;
+
 			@Override
 			protected boolean doCheckState() {
 				GDBDynamicPrintfUtils.GDBDynamicPrintfString parsedStr = new GDBDynamicPrintfUtils.GDBDynamicPrintfString(
@@ -317,6 +321,30 @@ protected boolean doCheckState() {
 
 				return valid;
 			}
+
+			@Override
+			public int getNumberOfControls() {
+				return super.getNumberOfControls() + 1;
+			}
+
+			@Override
+			protected void adjustForNumColumns(int numColumns) {
+				super.adjustForNumColumns(numColumns - 1);
+			}
+
+			@Override
+			protected void doFillIntoGrid(Composite parent, int numColumns) {
+				super.doFillIntoGrid(parent, numColumns - 1);
+				if (closingParenLabel == null) {
+					closingParenLabel = new Label(parent, SWT.LEFT);
+					closingParenLabel.setFont(parent.getFont());
+					closingParenLabel.setText(")"); //$NON-NLS-1$
+					closingParenLabel.addDisposeListener(event -> closingParenLabel = null);
+				} else {
+					checkParent(closingParenLabel, parent);
+				}
+
+			}
 		};
 		addField(fPrintString);
 	}

diff --git a/...rg.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/GDBDynamicPrintfUtils.java b/...rg.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/GDBDynamicPrintfUtils.java
@@ -15,7 +15,6 @@
 package org.eclipse.cdt.dsf.gdb.breakpoints;
 
 import org.eclipse.cdt.dsf.concurrent.Immutable;
-import org.eclipse.osgi.util.NLS;
 
 /**
  * Utility methods to help deal with Dynamic Printf logic.
@@ -51,17 +50,10 @@ public GDBDynamicPrintfString(String str) {
 				return;
 			}
 
-			if (str.charAt(str.length() - 1) == ')') {
-				fErrorMessage = Messages.DynamicPrintf_Printf_not_expecting_a_closing_parenthesis;
-				return;
-			}
-
-			// Now go through the string and look for two things:
-			//   1- count the number of % (but ignore any %%)
-			//   2- find the closing double-quote (but ignore any \")
+			// Now go through the string and find the closing
+			// double-quote (but ignore any \")
 			char[] chars = str.toCharArray();
 			int closingQuoteIndex = 0;
-			int numArgExpected = 0;
 			for (int i = 1; i < chars.length; i++) {
 				switch (chars[i]) {
 				case '\\':
@@ -72,11 +64,6 @@ public GDBDynamicPrintfString(String str) {
 					closingQuoteIndex = i;
 					break;
 				case '%':
-					if (chars.length > i + 1 && chars[i + 1] != '%') {
-						// Not a %% so we have found an expected argument.
-						numArgExpected++;
-					}
-
 					// We either found a second %, which we must skip to
 					// avoid parsing it again, or we didn't and we know
 					// our next character must be part of the format.
@@ -101,40 +88,21 @@ public GDBDynamicPrintfString(String str) {
 			// We extract the string part of the printf string leaving the arguments
 			fStringSection = str.substring(0, closingQuoteIndex + 1);
 
-			int numArgPresent = 0;
 			if (closingQuoteIndex + 1 >= str.length()) {
 				// No more characters after the string part
 				fArgs = new String[0];
-				numArgPresent = 0;
 			} else {
 				String argString = str.substring(closingQuoteIndex + 1).trim();
+
 				if (argString.charAt(0) != ',') {
 					fErrorMessage = Messages.DynamicPrintf_Missing_comma;
 					return;
 				}
 
-				// Remove the first , to avoid an empty element after the split.
-				// Then split the string but keep any empty results
-				String[] args = argString.substring(1).split(",", -1); //$NON-NLS-1$
-
-				for (String argument : args) {
-					if (argument.trim().isEmpty()) {
-						fErrorMessage = Messages.DynamicPrintf_Empty_arg;
-						return;
-					}
-				}
-
-				fArgs = args;
-				numArgPresent = fArgs.length;
-			}
-
-			if (numArgPresent != numArgExpected) {
-				if (numArgPresent > numArgExpected) {
-					fErrorMessage = NLS.bind(Messages.DynamicPrintf_Extra_arg, numArgPresent - numArgExpected);
-				} else {
-					fErrorMessage = NLS.bind(Messages.DynamicPrintf_Missing_arg, numArgExpected - numArgPresent);
-				}
-				return;
+				// Remove the first , that separates the format
+				// from the arguments
+				String printfArguments = argString.substring(1);
+				fArgs = new String[] { printfArguments };
 			}
 
 			// Everything is ok!

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/Messages.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/Messages.java
@@ -22,14 +22,7 @@ public class Messages extends NLS {
 	public static String DynamicPrintf_Invalid_string;
 	public static String DynamicPrintf_Printf_must_start_with_quote;
 	public static String DynamicPrintf_Printf_missing_closing_quote;
-	/**
-	 * @since 5.3
-	 */
-	public static String DynamicPrintf_Printf_not_expecting_a_closing_parenthesis;
 	public static String DynamicPrintf_Missing_comma;
-	public static String DynamicPrintf_Empty_arg;
-	public static String DynamicPrintf_Extra_arg;
-	public static String DynamicPrintf_Missing_arg;
 
 	static {
 		// initialize resource bundle

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/Messages.properties b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/Messages.properties
@@ -15,8 +15,4 @@
 DynamicPrintf_Invalid_string=Invalid printf string
 DynamicPrintf_Printf_must_start_with_quote=Printf string must start with a "
 DynamicPrintf_Printf_missing_closing_quote=Printf string is missing its closing "
-DynamicPrintf_Printf_not_expecting_a_closing_parenthesis=Specify Printf parameters without a closing parenthesis
 DynamicPrintf_Missing_comma=Printf string can only have a , after its closing "
-DynamicPrintf_Empty_arg=Printf string should not have empty arguments
-DynamicPrintf_Extra_arg={0} extra arguments in printf specification
-DynamicPrintf_Missing_arg={0} missing arguments in printf specification