[v3] sideband.c: remove redundant 'NEEDSWORK' tag

Commit Message

Chandra Pratap Dec. 27, 2023, 10:20 a.m. UTC

From: Chandra Pratap <[e-mail]>

Signed-off-by: Chandra Pratap <[e-mail]>
---
    sideband.c: replace int with size_t for clarity

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1625%2FChand-ra%2Fdusra-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1625/Chand-ra/dusra-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1625

Range-diff vs v2:

 1:  fdacc69ae3b ! 1:  273415aa6a4 sideband.c: replace int with size_t for clarity
     @@ Metadata
      Author: Chandra Pratap <[e-mail]>
      
       ## Commit message ##
     -    sideband.c: replace int with size_t for clarity
     -
     -    Replace int with size_t for non-negative values to improve
     -    clarity and remove the 'NEEDSWORK' tag associated with it.wq
     +    sideband.c: remove redundant 'NEEDSWORK' tag
      
          Signed-off-by: Chandra Pratap <[e-mail]>
      
     @@ sideband.c: void list_config_color_sideband_slots(struct string_list *list, cons
        * passed as the first N characters of the SRC array.
        *
      - * NEEDSWORK: use "size_t n" instead for clarity.
     ++ * It is fine to use "int n" here instead of "size_t n" as all calls to this
     ++ * function pass an 'int' parameter.
        */
     --static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
     -+static void maybe_colorize_sideband(struct strbuf *dest, const char *src, size_t n)
     + static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
       {
     - 	int i;
     - 
     -@@ sideband.c: static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
     - 
     - 	for (i = 0; i < ARRAY_SIZE(keywords); i++) {
     - 		struct keyword_entry *p = keywords + i;
     --		int len = strlen(p->keyword);
     -+		size_t len = strlen(p->keyword);
     - 
     - 		if (n < len)
     - 			continue;


 sideband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


base-commit: 1a87c842ece327d03d08096395969aca5e0a6996

Comments

Junio C Hamano Dec. 27, 2023, 10:22 p.m. UTC | #1

"Chandra Pratap via GitGitGadget" <[e-mail]> writes:

>       - * NEEDSWORK: use "size_t n" instead for clarity.
>      ++ * It is fine to use "int n" here instead of "size_t n" as all calls to this
>      ++ * function pass an 'int' parameter.

This does not sound like a sufficient justification, though.

We should also explain why "int" is good enough for these callers.
Otherwise, using size_t throughout the callchain would become
another viable solution.

Patch

diff --git a/sideband.c b/sideband.c
index 6cbfd391c47..a89201d52ac 100644
--- a/sideband.c
+++ b/sideband.c
@@ -69,7 +69,8 @@  void list_config_color_sideband_slots(struct string_list *list, const char *pref
  * of the line. This should be called for a single line only, which is
  * passed as the first N characters of the SRC array.
  *
- * NEEDSWORK: use "size_t n" instead for clarity.
+ * It is fine to use "int n" here instead of "size_t n" as all calls to this
+ * function pass an 'int' parameter.
  */
 static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 {