Skip to content

Suppress some minor analysis warnings #593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NWilson merged 1 commit into from
Dec 7, 2024
Merged

Suppress some minor analysis warnings #593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/pcre2_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7285,7 +7285,6 @@ for (;; pptr++)
single-char opcodes. */

reqvary = (repeat_min == repeat_max)? 0 : REQ_VARY;
op_type = 0;
Copy link
Member Author

@NWilson NWilson Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clang is correct here. The assignment to op_type is provably a dead assignment. What's more... I reckon it hurts rather than helps, because if there's a branch in the code that should be assigning to op_type but forgets, this would suppress warnings about use-of-uninitialised!


/* Adjust first and required code units for a zero repeat. */

Expand Down Expand Up @@ -7848,7 +7847,8 @@ for (;; pptr++)
}
else
{
/* Come here from just above with a character in mcbuffer/mclength. */
/* Come here from just above with a character in mcbuffer/mclength.
You must also set op_type before the jump. */
OUTPUT_SINGLE_REPEAT:
prop_type = prop_value = -1;
}
Expand Down
3 changes: 3 additions & 0 deletions src/pcre2_compile_class.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ while (c <= end)
buffer[0] = new_start;
buffer[1] = new_end;
buffer += 2;
(void)buffer;
Copy link
Member Author

@NWilson NWilson Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have some clearly useless assignments here, but they're OK to keep, because they ensure the buffer is correctly tracked if someone extends the function later. Easy suppression.

}
return result;
}
Expand Down Expand Up @@ -330,6 +331,7 @@ while (*p != NOTACHAR)
buffer[0] = start;
buffer[1] = get_highest_char(options);
buffer += 2;
(void)buffer;
}

return result;
Expand Down Expand Up @@ -745,6 +747,7 @@ for (c = 0; c < 256; c++)
{
prop = GET_UCD(c);
set_bit = FALSE;
(void)set_bit;

switch (ptype)
{
Expand Down
2 changes: 1 addition & 1 deletion src/pcre2_dfa_match.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3070,7 +3070,7 @@ for (;;)
if (codevalue == OP_BRAPOSZERO)
{
allow_zero = TRUE;
codevalue = *(++code); /* Codevalue will be one of above BRAs */
++code; /* The following opcode will be one of the above BRAs */
Copy link
Member Author

@NWilson NWilson Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clang is correct. We are writing a value to codevalue which is provably never used.

}
else allow_zero = FALSE;

Expand Down
7 changes: 5 additions & 2 deletions src/pcre2grep.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3059,7 +3059,6 @@ while (ptr < endptr)
if (group_separator != NULL)
fprintf(stdout, "%s%s", group_separator, STDOUT_NL);
hyphenpending = FALSE;
hyphenprinted = TRUE;
}

/* Now print the matching line(s); ensure we set hyphenpending at the end
Expand Down Expand Up @@ -3874,8 +3873,12 @@ else
filename = name;
}

while ((patlen = sizeof(buffer)) && read_pattern(buffer, &patlen, f))
while (TRUE)
{
patlen = sizeof(buffer);
if (!read_pattern(buffer, &patlen, f))
break;

if (!posix_pattern_file)
{
while (patlen > 0 && isspace((unsigned char)(buffer[patlen-1]))) patlen--;
Expand Down
5 changes: 4 additions & 1 deletion src/pcre2posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ changed. This #define is a copy of the one in pcre2_internal.h. */

#include "pcre2.h"
#include "pcre2posix.h"
#include "pcre2_util.h"

/* Table to translate PCRE2 compile time error codes into POSIX error codes.
Only a few PCRE2 errors with a value greater than 23 turn into special POSIX
Expand Down Expand Up @@ -194,7 +195,7 @@ if (preg != NULL && (int)preg->re_erroffset != -1)
/* no need to deal with UB in snprintf */
if (errbuf_size > INT_MAX) errbuf_size = INT_MAX;

/* there are 11 charactes between message and offset,
/* there are 11 characters between message and offset;
update message_len() if changed */
ret = snprintf(errbuf, errbuf_size, "%s at offset %d", message,
(int)preg->re_erroffset);
Expand All @@ -210,6 +211,8 @@ else
ret = (int)len;
}

PCRE2_ASSERT(len > 0 || preg != NULL);
Copy link
Member Author

@NWilson NWilson Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an assertion here lets Clang reason about when preg is NULL. There was no bug, but static analysis wasn't able to work out that checking len was equivalent to checking preg != NULL.


do {
if (ret < 0)
{
Expand Down
Loading