threads / discuss / 56703

Possibly wrong assignment in config.c

Subject: Possibly wrong assignment in config.c

## tl;dr

2 messages between Oct 13, 2021 and Oct 13, 2021.

replies: 1people: 2as markdown or json

Andrea Monaco· Oct 13, 2021, 17:05 UTC · lore
Hello,
while building git I get the following warning:
config.c: In function 'git_config_copy_or_rename_section_in_file':
config.c:3358:17: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
 3358 |       output[0] = '\t';
      |       ~~~~~~~~~~^~~~~~
config.c:3264:7: note: at offset -1 to object 'buf' with size 1024 declared here
 3264 |  char buf[1024];
      |       ^~~
concerning these lines:
  output -= 1;
  output[0] = '\t';

Perhaps it has some merit. output is previously equal to an array buf[1024], so the assignment apparently goes outside allocated memory.

Let me know,
Andrea Monaco
Junio C Hamano· Oct 13, 2021, 19:14 UTC · re: Andrea Monaco · lore

Re: Possibly wrong assignment in config.c

Andrea Monaco <andrea.monaco@autistici.org> writes:
> config.c: In function 'git_config_copy_or_rename_section_in_file':
> config.c:3358:17: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
>  3358 |       output[0] = '\t';
>       |       ~~~~~~~~~~^~~~~~

Hmph, with a wider context, I think the tool is pointing at the assignment found here?

	while (fgets(buf, sizeof(buf), config_file)) {
		unsigned i;
		int length;
		int is_section = 0;
		char *output = buf;
		for (i = 0; buf[i] && isspace(buf[i]); i++)
			; /* do nothing */
		if (buf[i] == '[') {
			/* it's a section */
			int offset;
			...
			offset = section_name_match(&buf[i], old_name);
			if (offset > 0) {
				...
					output += offset + i;
					if (strlen(output) > 0) {
						...
						output -= 1;
						output[0] = '\t';
					}
				} else {
					copystr = store_create_section(new_name, &store);
				}
			}
			remove = 0;
		}

Inside the "if (buf[i] == '[')" block, i is not negative, and inside the "if (offset > 0)" block, offset is positive. So "output += offset + i", unless offset and i are so huge to cause integer wraparound, would only be incrementing offset by some positive integer. So immediately after "output += offset + i", output is at least buf+1, if not larger, and &output[-1] is at least pointing at &buf[0], no?

Or is the tool worried about integer addition (offset+i) wrapping around?

Or I may need a bit more caffeine, perhaps?  I am puzzled...
Thanks.

← back to recent threads