[14/15] cleanup: move parsing helpers to parse.c

Commit Message

Luc Van Oostenryck July 5, 2020, 1:02 p.m. UTC

lib.c contains 2-3 helpers for parsing. Move them to parse.c.

Signed-off-by: Luc Van Oostenryck <[e-mail]>
---
 lib.c   | 38 --------------------------------------
 parse.c | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 38 deletions(-)

Comments

Linus Torvalds July 5, 2020, 5:27 p.m. UTC | #1

On Sun, Jul 5, 2020 at 6:02 AM Luc Van Oostenryck
<[e-mail]> wrote:
>
> lib.c contains 2-3 helpers for parsing. Move them to parse.c.

This makes sense, because it's not a "library" function if it's only
used in one place.

HOWEVER.

When doing things like this, please also mark the resulting function
static and remove the declaration from lib.h.

Otherwise it's entirely pointless, I feel.

Either it's a library function that gets used from other places (and
lib.c/lib.h is an appropriate place), or it's a parsing-only helper
function that _doesn't_ get used from other places (and it should be
moved to parse.c and be static).

Not this half-way state that this patch seems to create.

               Linus

Luc Van Oostenryck July 5, 2020, 8:45 p.m. UTC | #2

On Sun, Jul 05, 2020 at 10:27:36AM -0700, Linus Torvalds wrote:
> On Sun, Jul 5, 2020 at 6:02 AM Luc Van Oostenryck
> <[e-mail]> wrote:
> >
> > lib.c contains 2-3 helpers for parsing. Move them to parse.c.
> 
> This makes sense, because it's not a "library" function if it's only
> used in one place.
> 
> HOWEVER.
> 
> When doing things like this, please also mark the resulting function
> static and remove the declaration from lib.h.
> 
> Otherwise it's entirely pointless, I feel.
> 
> Either it's a library function that gets used from other places (and
> lib.c/lib.h is an appropriate place), or it's a parsing-only helper
> function that _doesn't_ get used from other places (and it should be
> moved to parse.c and be static).
> 
> Not this half-way state that this patch seems to create.

Yes, I agree.
I see them as parsing-only helpers hence the move but (just)
expect() is also used in expression.c. I suppose it should be
fine to move the declaration to parse.h despite not being ideal.
Or maybe, I should just leave them in lib.c

-- Luc

Patch

diff --git a/lib.c b/lib.c
index fd1fe6cb3ba5..f512be2e1a43 100644
--- a/lib.c
+++ b/lib.c
@@ -50,44 +50,6 @@ 
 #include "bits.h"
 
 
-struct token *skip_to(struct token *token, int op)
-{
-	while (!match_op(token, op) && !eof_token(token))
-		token = token->next;
-	return token;
-}
-
-static struct token bad_token = { .pos.type = TOKEN_BAD };
-struct token *expect(struct token *token, int op, const char *where)
-{
-	if (!match_op(token, op)) {
-		if (token != &bad_token) {
-			bad_token.next = token;
-			sparse_error(token->pos, "Expected %s %s", show_special(op), where);
-			sparse_error(token->pos, "got %s", show_token(token));
-		}
-		if (op == ';')
-			return skip_to(token, op);
-		return &bad_token;
-	}
-	return token->next;
-}
-
-///
-// issue an error message on new parsing errors
-// @token: the current token
-// @errmsg: the error message
-// If the current token is from a previous error, an error message
-// has already been issued, so nothing more is done.
-// Otherwise, @errmsg is displayed followed by the current token.
-void unexpected(struct token *token, const char *errmsg)
-{
-	if (token == &bad_token)
-		return;
-	sparse_error(token->pos, "%s", errmsg);
-	sparse_error(token->pos, "got %s", show_token(token));
-}
-
 unsigned int hexval(unsigned int c)
 {
 	int retval = 256;
diff --git a/parse.c b/parse.c
index 70d8b237ce5e..cea208395090 100644
--- a/parse.c
+++ b/parse.c
@@ -655,6 +655,44 @@  void init_parser(int stream)
 }
 
 
+struct token *skip_to(struct token *token, int op)
+{
+	while (!match_op(token, op) && !eof_token(token))
+		token = token->next;
+	return token;
+}
+
+static struct token bad_token = { .pos.type = TOKEN_BAD };
+struct token *expect(struct token *token, int op, const char *where)
+{
+	if (!match_op(token, op)) {
+		if (token != &bad_token) {
+			bad_token.next = token;
+			sparse_error(token->pos, "Expected %s %s", show_special(op), where);
+			sparse_error(token->pos, "got %s", show_token(token));
+		}
+		if (op == ';')
+			return skip_to(token, op);
+		return &bad_token;
+	}
+	return token->next;
+}
+
+///
+// issue an error message on new parsing errors
+// @token: the current token
+// @errmsg: the error message
+// If the current token is from a previous error, an error message
+// has already been issued, so nothing more is done.
+// Otherwise, @errmsg is displayed followed by the current token.
+void unexpected(struct token *token, const char *errmsg)
+{
+	if (token == &bad_token)
+		return;
+	sparse_error(token->pos, "%s", errmsg);
+	sparse_error(token->pos, "got %s", show_token(token));
+}
+
 // Add a symbol to the list of function-local symbols
 static void fn_local_symbol(struct symbol *sym)
 {