Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Small patch for out of bounds warning.

(1/2) > >>

Smitty:
Here is a little patch to correct a warning - a legitimate warning.
I looked at submitting a patch in the prescribed fashion, but frankly, the instructions were so complicated, that I decided I couldn't be bothered.
I think that should be addressed - it should be mutch easier for anyone to submit patches, or merge requests.


--- Code: ---src/scintilla/lexers/LexMMIXAL.cxx

Index: wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx
===================================================================
--- wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx (revision 12450)
+++ wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx (working copy)
@@ -103,7 +103,7 @@
  char s[100];
  sc.GetCurrent(s, sizeof(s));
  if (*s == ':') { // ignore base prefix for match
-                                       for (size_t i = 0; i != sizeof(s); ++i) {
+                                       for (size_t i = 0; i != sizeof(s[0]); ++i) {
  *(s+i) = *(s+i+1);
                                        }
  }

--- End code ---

sodev:
Not sure what the warning was, most probably about accessing an index out of bounds, but your patch does not fix the code, it actually breaks the code. The code fragment removes a leading colon from a string by shifting down the content by one, sizeof(s) == 100, sizeof(s[0]) == 1, after your patch the colon gets overwritten with the following character only. A proper fix would be to use sizeof(s) - 1 in the condition.

Miguel Gimenez:
Or use
--- Code: ---memmove(s, s+1, sizeof(s)-1)
--- End code ---

omlk:

--- Quote from: Miguel Gimenez on May 20, 2021, 10:18:14 am ---Or use
--- Code: ---memmove(s, s+1, sizeof(s)-1)
--- End code ---

--- End quote ---


--- Code: ---
s[100] = "123456789";
s[0] = '1'; ...; s[8] = '9'; s[last] = '\0';

char s[100]; //WCHAR s[100];
char s2[1000];
unsigned char_array_size_bytes = sizeof(s);
unsigned char char_size_bytes = sizeof(s[0]);
unsigned char_counts = char_array_size_bytes /char_size_bytes;
memmove(s2+5, s, strlen(s)+1)
--- End code ---

* https://www.geeksforgeeks.org/write-memcpy/

Smitty:
I had some problems with pasting my patch into this forum, and somehow I corrupted it - I apologize, I didn't notice.

Absolutely correct, that I have broken almost correct code.

Aside from the fact that there are better ways of doing this, it will cause a buffer over-run warning (on most standard compilers)

Granted, at compile time the sizeof(s) is computed correctly - since s is stack based - this is not an error.  But, warnings should not be allowed to persist.

Navigation

[0] Message Index

[#] Next page