hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 17:25:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Force LF for .c,.cpp,.h,.hpp

Browse Source
This commit is contained in:
Dave Bartolomeo
2018-09-20 11:38:56 -07:00
parent caf4a767ad
commit aa267c8302
200 changed files with 5422 additions and 5417 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyDependencies.cpp
View File

@@ -1,17 +1,17 @@
// an include declaration just adds one source dependency, it does not automatically
// add a dependency from this file to all the declarations in stdio.h
#include <stdio.h>
#include <myfile.h> // contains non-static global myfile_err
extern int myfile_err; // this external declaration adds a dependency on myfile.h
class C {
public:
C() {
// one dependency for printf:
printf("Hello world!");
// one dependency for FILE type, and one for NULL macro:
FILE fp = NULL;
}
};
// an include declaration just adds one source dependency, it does not automatically
// add a dependency from this file to all the declarations in stdio.h
#include <stdio.h>
#include <myfile.h> // contains non-static global myfile_err
extern int myfile_err; // this external declaration adds a dependency on myfile.h
class C {
public:
C() {
// one dependency for printf:
printf("Hello world!");
// one dependency for FILE type, and one for NULL macro:
FILE fp = NULL;
}
};

40
cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyFields.cpp
View File

@@ -1,20 +1,20 @@
//This struct contains 30 fields.
struct MyParticle {
bool isActive;
int priority;
float x, y, z;
float dx, dy, dz;
float ddx, ddy, ddz;
bool isCollider;
int age, maxAge;
float size1, size2;
bool hasColor;
unsigned char r1, g1, b1, a1;
unsigned char r2, g2, b2, a2;
class texture *tex;
float u1, v1, u2, v2;
};
//This struct contains 30 fields.
struct MyParticle {
bool isActive;
int priority;
float x, y, z;
float dx, dy, dz;
float ddx, ddy, ddz;
bool isCollider;
int age, maxAge;
float size1, size2;
bool hasColor;
unsigned char r1, g1, b1, a1;
unsigned char r2, g2, b2, a2;
class texture *tex;
float u1, v1, u2, v2;
};

16
cpp/ql/src/Architecture/Refactoring Opportunities/FunctionsWithManyParameters.cpp
View File

@@ -1,8 +1,8 @@
// this example has 15 parameters.
void fillRect(int x, int y, int w, int h,
int r1, int g1, int b1, int a1,
int r2, int g2, int b2, int a2,
gradient_type grad, unsigned int flags, bool border)
{
// ...
}
// this example has 15 parameters.
void fillRect(int x, int y, int w, int h,
int r1, int g1, int b1, int a1,
int r2, int g2, int b2, int a2,
gradient_type grad, unsigned int flags, bool border)
{
// ...
}

24
cpp/ql/src/Best Practices/ComplexCondition.cpp
View File

@@ -1,13 +1,13 @@
//This condition is too complex and can be improved by using local variables
bool accept_message =
(message_type == CONNECT && _state != CONNECTED) ||
(message_type == DISCONNECT && _state == CONNECTED) ||
(message_type == DATA && _state == CONNECTED);
//This condition is acceptable, as all the logical operators are of the same type (&&)
bool valid_connect =
message_type == CONNECT &&
_state != CONNECTED &&
time_since_prev_connect > MAX_CONNECT_INTERVAL &&
message_length <= MAX_PACKET_SIZE &&
//This condition is too complex and can be improved by using local variables
bool accept_message =
(message_type == CONNECT && _state != CONNECTED) ||
(message_type == DISCONNECT && _state == CONNECTED) ||
(message_type == DATA && _state == CONNECTED);
//This condition is acceptable, as all the logical operators are of the same type (&&)
bool valid_connect =
message_type == CONNECT &&
_state != CONNECTED &&
time_since_prev_connect > MAX_CONNECT_INTERVAL &&
message_length <= MAX_PACKET_SIZE &&
checksum(message) == get_checksum_field(message);

12
cpp/ql/src/Best Practices/Hiding/DeclarationHidesParameter.cpp
View File

@@ -1,6 +1,6 @@
void f(int i) {
for (int i = 0; i < 10; ++i) { //the loop variable hides the parameter to f()
...
}
}
void f(int i) {
for (int i = 0; i < 10; ++i) { //the loop variable hides the parameter to f()
...
}
}

24
cpp/ql/src/Best Practices/Hiding/DeclarationHidesVariable.cpp
View File

@@ -1,12 +1,12 @@
void f() {
int i = 10;
for (int i = 0; i < 10; i++) { //the loop counter hides the variable
...
}
{
int i = 12; //this variable hides the variable in the outer block
...
}
}
void f() {
int i = 10;
for (int i = 0; i < 10; i++) { //the loop counter hides the variable
...
}
{
int i = 12; //this variable hides the variable in the outer block
...
}
}

24
cpp/ql/src/Best Practices/Hiding/LocalVariableHidesGlobalVariable.cpp
View File

@@ -1,12 +1,12 @@
int i = 10;
void f() {
for (int i = 0; i < 10; i++) { //the loop counter hides the global variable i
...
}
{
int i = 12; //this variable hides the global variable i
...
}
}
int i = 10;
void f() {
for (int i = 0; i < 10; i++) { //the loop counter hides the global variable i
...
}
{
int i = 12; //this variable hides the global variable i
...
}
}

18
cpp/ql/src/Best Practices/Likely Errors/EmptyBlock.cpp
View File

@@ -1,9 +1,9 @@
void f(int i) {
if (i == 10); //empty then block
... //won't be part of the if statement
if (i == 12) {
...
} else { //empty else block, most likely a mistake
}
}
void f(int i) {
if (i == 10); //empty then block
... //won't be part of the if statement
if (i == 12) {
...
} else { //empty else block, most likely a mistake
}
}

86
cpp/ql/src/Best Practices/Likely Errors/Slicing.cpp
View File

@@ -1,43 +1,43 @@
static int idctr = 0;
//Basic connection with id
class Connection {
public:
int connId;
virtual void print_info() {
cout << "id: " << connId << "\n";
}
Connection() {
connId = idctr++;
}
};
//Adds counters, and an overriding print_info
class MeteredConnection : public Connection {
public:
int txCtr;
int rxCtr;
MeteredConnection() {
txCtr = 0;
rxCtr = 0;
}
virtual void print_info() {
cout << "id: " << connId << "\n" << "tx/rx: " << txCtr << "/" << rxCtr << "\n";
}
};
int main(int argc, char* argv[]) {
Connection conn;
MeteredConnection m_conn;
Connection curr_conn = conn;
curr_conn.print_info();
curr_conn = m_conn; //Wrong: Derived MetricConnection assigned to Connection
//variable, will slice off the counters and the overriding print_info
curr_conn.print_info(); //Will not print the counters.
Connection* curr_pconn = &conn;
curr_pconn->print_info();
curr_pconn = &m_conn; //Correct: Pointer assigned to address of the MetricConnection.
//Counters and virtual functions remain intact.
curr_pconn->print_info(); //Will call the correct method MeteredConnection::print_info
}
static int idctr = 0;
//Basic connection with id
class Connection {
public:
int connId;
virtual void print_info() {
cout << "id: " << connId << "\n";
}
Connection() {
connId = idctr++;
}
};
//Adds counters, and an overriding print_info
class MeteredConnection : public Connection {
public:
int txCtr;
int rxCtr;
MeteredConnection() {
txCtr = 0;
rxCtr = 0;
}
virtual void print_info() {
cout << "id: " << connId << "\n" << "tx/rx: " << txCtr << "/" << rxCtr << "\n";
}
};
int main(int argc, char* argv[]) {
Connection conn;
MeteredConnection m_conn;
Connection curr_conn = conn;
curr_conn.print_info();
curr_conn = m_conn; //Wrong: Derived MetricConnection assigned to Connection
//variable, will slice off the counters and the overriding print_info
curr_conn.print_info(); //Will not print the counters.
Connection* curr_pconn = &conn;
curr_pconn->print_info();
curr_pconn = &m_conn; //Correct: Pointer assigned to address of the MetricConnection.
//Counters and virtual functions remain intact.
curr_pconn->print_info(); //Will call the correct method MeteredConnection::print_info
}

32
cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.cpp
View File

@@ -1,16 +1,16 @@
void sanitize(Fields[] record) {
//The number of fields here can be put in a const
for (fieldCtr = 0; field < 7; field++) {
sanitize(fields[fieldCtr]);
}
}
#define NUM_FIELDS 7
void process(Fields[] record) {
//This avoids using a magic constant by using the macro instead
for (fieldCtr = 0; field < NUM_FIELDS; field++) {
process(fields[fieldCtr]);
}
}
void sanitize(Fields[] record) {
//The number of fields here can be put in a const
for (fieldCtr = 0; field < 7; field++) {
sanitize(fields[fieldCtr]);
}
}
#define NUM_FIELDS 7
void process(Fields[] record) {
//This avoids using a magic constant by using the macro instead
for (fieldCtr = 0; field < NUM_FIELDS; field++) {
process(fields[fieldCtr]);
}
}

52
cpp/ql/src/Best Practices/RuleOfTwo.cpp
View File

@@ -1,26 +1,26 @@
class C {
private:
Other* other = NULL;
public:
C(const C& copyFrom) {
Other* newOther = new Other();
*newOther = copyFrom.other;
this->other = newOther;
}
//No operator=, by default will just copy the pointer other, will not create a new object
};
class D {
Other* other = NULL;
public:
D& operator=(D& rhs) {
Other* newOther = new Other();
*newOther = rhs.other;
this->other = newOther;
return *this;
}
//No copy constructor, will just copy the pointer other and not create a new object
};
class C {
private:
Other* other = NULL;
public:
C(const C& copyFrom) {
Other* newOther = new Other();
*newOther = copyFrom.other;
this->other = newOther;
}
//No operator=, by default will just copy the pointer other, will not create a new object
};
class D {
Other* other = NULL;
public:
D& operator=(D& rhs) {
Other* newOther = new Other();
*newOther = rhs.other;
this->other = newOther;
return *this;
}
//No copy constructor, will just copy the pointer other and not create a new object
};

62
cpp/ql/src/Best Practices/SwitchLongCase.cpp
View File

@@ -1,32 +1,32 @@
//This switch statement has long case statements, and can become difficult to
//read as the processing for each message type becomes more complex
switch (message_type) {
case CONNECT:
_state = CONNECTING;
int message_id = message_get_id(message);
int source = connect_get_source(message);
//More code here...
send(connect_response);
break;
case DISCONNECT:
_state = DISCONNECTING;
int message_id = message_get_id(message);
int source = disconnect_get_source(message);
//More code here...
send(disconnect_response);
break;
default:
log("Invalid message, id : %d", message_get_id(message));
}
//This is better, as each case is split out to a separate function
switch (packet_type) {
case STREAM:
process_stream_packet(packet);
break;
case DATAGRAM:
process_datagram_packet(packet);
break;
default:
log("Invalid packet type: %d", packet_type);
//This switch statement has long case statements, and can become difficult to
//read as the processing for each message type becomes more complex
switch (message_type) {
case CONNECT:
_state = CONNECTING;
int message_id = message_get_id(message);
int source = connect_get_source(message);
//More code here...
send(connect_response);
break;
case DISCONNECT:
_state = DISCONNECTING;
int message_id = message_get_id(message);
int source = disconnect_get_source(message);
//More code here...
send(disconnect_response);
break;
default:
log("Invalid message, id : %d", message_get_id(message));
}
//This is better, as each case is split out to a separate function
switch (packet_type) {
case STREAM:
process_stream_packet(packet);
break;
case DATAGRAM:
process_datagram_packet(packet);
break;
default:
log("Invalid packet type: %d", packet_type);
}

10
cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.cpp
View File

@@ -1,5 +1,5 @@
{
int x = 0; //x is unused
int y = 0;
cout << y;
}
{
int x = 0; //x is unused
int y = 0;
cout << y;
}

28
cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.cpp
View File

@@ -1,14 +1,14 @@
//start of file
static void f() { //static function f() is unused in the file
//...
}
static void g() {
//...
}
void public_func() { //non-static function public_func is not called in file,
//but could be visible in other files
//...
g(); //call to g()
//...
}
//end of file
//start of file
static void f() { //static function f() is unused in the file
//...
}
static void g() {
//...
}
void public_func() { //non-static function public_func is not called in file,
//but could be visible in other files
//...
g(); //call to g()
//...
}
//end of file

10
cpp/ql/src/Best Practices/Unused Entities/UnusedStaticVariables.cpp
View File

@@ -1,5 +1,5 @@
void f() {
static int i = 0; //i is unused
...
return;
}
void f() {
static int i = 0; //i is unused
...
return;
}

38
cpp/ql/src/Critical/DeadCodeCondition.cpp
View File

@@ -1,19 +1,19 @@
while(result) {
if ( ... )
...
else if (result //wrong: this test is redundant
&& result->flags != 0)
...
result = next(queue);
}
fp = fopen(log, "r");
if (fp) {
/*
* large block of code
*/
if (!fp) { //wrong: always false
... /* dead code */
}
}
while(result) {
if ( ... )
...
else if (result //wrong: this test is redundant
&& result->flags != 0)
...
result = next(queue);
}
fp = fopen(log, "r");
if (fp) {
/*
* large block of code
*/
if (!fp) { //wrong: always false
... /* dead code */
}
}

24
cpp/ql/src/Critical/DeadCodeFunction.cpp
View File

@@ -1,12 +1,12 @@
class C {
public:
void g() {
...
//f() was previously used but is now commented, orphaning f()
//f();
...
}
private:
void f() { //is now unused, and can be removed
}
};
class C {
public:
void g() {
...
//f() was previously used but is now commented, orphaning f()
//f();
...
}
private:
void f() { //is now unused, and can be removed
}
};

18
cpp/ql/src/Critical/DescriptorMayNotBeClosed.cpp
View File

@@ -1,9 +1,9 @@
int f() {
try {
int sockfd = socket(AF_INET, SOCK_STREAM, 0);
do_stuff(sockfd);
return sockfd; //if there are no exceptions, the socket is returned
} catch (int do_stuff_exception) {
return -1; //return error value, but sockfd may still be open
}
}
int f() {
try {
int sockfd = socket(AF_INET, SOCK_STREAM, 0);
do_stuff(sockfd);
return sockfd; //if there are no exceptions, the socket is returned
} catch (int do_stuff_exception) {
return -1; //return error value, but sockfd may still be open
}
}

12
cpp/ql/src/Critical/DescriptorNeverClosed.cpp
View File

@@ -1,6 +1,6 @@
int main(int argc, char* argv[]) {
int sockfd = socket(AF_INET, SOCK_STREAM, 0);
int status = 0;
... //code that does not close sockfd
return status; //sockfd is never closed
}
int main(int argc, char* argv[]) {
int sockfd = socket(AF_INET, SOCK_STREAM, 0);
int status = 0;
... //code that does not close sockfd
return status; //sockfd is never closed
}

22
cpp/ql/src/Critical/GlobalUseBeforeInit.cpp
View File

@@ -1,11 +1,11 @@
int g_callCtr;
void initGlobals() {
g_callCtr = 0;
}
int main(int argc, char* argv[]) {
...
cout << g_callCtr; //callCtr used before it is initialized
initGlobals();
}
int g_callCtr;
void initGlobals() {
g_callCtr = 0;
}
int main(int argc, char* argv[]) {
...
cout << g_callCtr; //callCtr used before it is initialized
initGlobals();
}

24
cpp/ql/src/Critical/InitialisationNotRun.cpp
View File

@@ -1,12 +1,12 @@
GlobalStorage *g_storage;
void init() { //initializes g_storage, but is never run from main
g_storage = new GlobalStorage();
...
}
int main(int argc, char *argv[]) {
... //init not called
strcpy(g_storage->name, argv[1]); // g_storage is used before init() is called
...
}
GlobalStorage *g_storage;
void init() { //initializes g_storage, but is never run from main
g_storage = new GlobalStorage();
...
}
int main(int argc, char *argv[]) {
... //init not called
strcpy(g_storage->name, argv[1]); // g_storage is used before init() is called
...
}

26
cpp/ql/src/Critical/LargeParameter.cpp
View File

@@ -1,13 +1,13 @@
typedef struct Names {
char first[100];
char last[100];
} Names;
int doFoo(Names n) { //wrong: n is passed by value (meaning the entire structure
//is copied onto the stack, instead of just a pointer)
...
}
int doBar(Names &n) { //better, only a reference is passed
...
}
typedef struct Names {
char first[100];
char last[100];
} Names;
int doFoo(Names n) { //wrong: n is passed by value (meaning the entire structure
//is copied onto the stack, instead of just a pointer)
...
}
int doBar(Names &n) { //better, only a reference is passed
...
}

22
cpp/ql/src/Critical/MissingNullTest.cpp
View File

@@ -1,11 +1,11 @@
typedef struct {
char name[100];
int status;
} person;
void f() {
person* buf = NULL;
buf = malloc(sizeof(person));
(*buf).status = 0; //access to buf before it was checked for NULL
}
typedef struct {
char name[100];
int status;
} person;
void f() {
person* buf = NULL;
buf = malloc(sizeof(person));
(*buf).status = 0; //access to buf before it was checked for NULL
}

10
cpp/ql/src/Critical/NewArrayDeleteMismatch.cpp
View File

@@ -1,5 +1,5 @@
Record* record = new Record[SIZE];
...
delete record; //record was created using 'new[]', but was freed using 'delete'
Record* record = new Record[SIZE];
...
delete record; //record was created using 'new[]', but was freed using 'delete'

10
cpp/ql/src/Critical/NewDeleteArrayMismatch.cpp
View File

@@ -1,5 +1,5 @@
Record *ptr = new Record(...);
...
delete [] ptr; // ptr was created using 'new', but was freed using 'delete[]'
Record *ptr = new Record(...);
...
delete [] ptr; // ptr was created using 'new', but was freed using 'delete[]'

10
cpp/ql/src/Critical/NewFreeMismatch.cpp
View File

@@ -1,5 +1,5 @@
Record *ptr = new Record(...);
...
free(ptr); // BAD: ptr was created using 'new', but is being freed using 'free'
Record *ptr = new Record(...);
...
free(ptr); // BAD: ptr was created using 'new', but is being freed using 'free'

12
cpp/ql/src/Critical/NotInitialised.cpp
View File

@@ -1,6 +1,6 @@
{
int i;
...
int g = COEFF * i; //i is used before it is initialized
}
{
int i;
...
int g = COEFF * i; //i is used before it is initialized
}

26
cpp/ql/src/Critical/OverflowDestination.cpp
View File

@@ -1,13 +1,13 @@
int main(int argc, char* argv[]) {
char param[SIZE];
char arg1[10];
char arg2[20];
//wrong: only uses the size of the source (argv[1]) when using strncpy
strncpy(param, argv[1], strlen(arg1));
//correct: uses the size of the destination array as well
strncpy(param, argv[1], min(strlen(arg1, sizeof(param) -1)));
}
int main(int argc, char* argv[]) {
char param[SIZE];
char arg1[10];
char arg2[20];
//wrong: only uses the size of the source (argv[1]) when using strncpy
strncpy(param, argv[1], strlen(arg1));
//correct: uses the size of the destination array as well
strncpy(param, argv[1], min(strlen(arg1, sizeof(param) -1)));
}

46
cpp/ql/src/Critical/ReturnValueIgnored.cpp
View File

@@ -1,23 +1,23 @@
int doFoo() {
...
return status;
}
void f() {
if (doFoo() == OK) {
...
}
}
void g() {
int status = doFoo();
if (status == OK) {
...
}
}
void err() {
doFoo(); //doFoo is called but its return value is not checked, and
//the value is checked in other locations
...
}
int doFoo() {
...
return status;
}
void f() {
if (doFoo() == OK) {
...
}
}
void g() {
int status = doFoo();
if (status == OK) {
...
}
}
void err() {
doFoo(); //doFoo is called but its return value is not checked, and
//the value is checked in other locations
...
}

20
cpp/ql/src/Critical/SizeCheck.cpp
View File

@@ -1,10 +1,10 @@
#define RECORD_SIZE 30 //incorrect or outdated size for record
typedef struct {
char name[30];
int status;
} Record;
void f() {
Record* p = malloc(RECORD_SIZE); //not of sufficient size to hold a Record
...
}
#define RECORD_SIZE 30 //incorrect or outdated size for record
typedef struct {
char name[30];
int status;
} Record;
void f() {
Record* p = malloc(RECORD_SIZE); //not of sufficient size to hold a Record
...
}

8
cpp/ql/src/Critical/Unused.cpp
View File

@@ -1,4 +1,4 @@
{
int foo = 1;
... //foo is unused
}
{
int foo = 1;
... //foo is unused
}

18
cpp/ql/src/Critical/UseAfterFree.cpp
View File

@@ -1,9 +1,9 @@
int f() {
char* buf = new char[SIZE];
....
if (error) {
free(buf); //error handling has freed the buffer
}
...
log_contents(buf); //but it is still used here for logging
}
int f() {
char* buf = new char[SIZE];
....
if (error) {
free(buf); //error handling has freed the buffer
}
...
log_contents(buf); //but it is still used here for logging
}

6
cpp/ql/src/Documentation/TodoComments.cpp
View File

@@ -1,4 +1,4 @@
int isOdd(int n) {
//TODO: Works only for positive n. Need to check if negative n is valid input
return (n % 2) == 1;
int isOdd(int n) {
//TODO: Works only for positive n. Need to check if negative n is valid input
return (n % 2) == 1;
}

14
cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.cpp
View File

@@ -1,8 +1,8 @@
// header_file.h
#ifndef HEADER_FILE_H
#define HEADER_FILE_H
// ...
// header_file.h
#ifndef HEADER_FILE_H
#define HEADER_FILE_H
// ...
#endif // HEADER_FILE_H

14
cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard2.cpp
View File

@@ -1,8 +1,8 @@
// another_header_file.h
#ifndef HEADER_FILE_H // should be ANOTHER_HEADER_FILE_H
#define HEADER_FILE_H // should be ANOTHER_HEADER_FILE_H
// ...
// another_header_file.h
#ifndef HEADER_FILE_H // should be ANOTHER_HEADER_FILE_H
#define HEADER_FILE_H // should be ANOTHER_HEADER_FILE_H
// ...
#endif // HEADER_FILE_H

12
cpp/ql/src/Likely Bugs/Arithmetic/ComparisonPrecedence.cpp
View File

@@ -1,6 +1,6 @@
void h() {
int a, b, c;
a < b != c; //parenthesize to explicitly define order of operators
(a < b) < c; //correct: parenthesized to specify order
}
void h() {
int a, b, c;
a < b != c; //parenthesize to explicitly define order of operators
(a < b) < c; //correct: parenthesized to specify order
}

24
cpp/ql/src/Likely Bugs/Conversion/ArrayArgSizeMismatch.cpp
View File

@@ -1,13 +1,13 @@
//Function foo's array parameter has a specified size
void foo(int a[10]) {
int i = 0;
for (i = 0; i <10; i++) {
a[i] = i * 2;
}
}
...
int my_arr[5];
//Function foo's array parameter has a specified size
void foo(int a[10]) {
int i = 0;
for (i = 0; i <10; i++) {
a[i] = i * 2;
}
}
...
int my_arr[5];
foo(my_arr); //my_arr is smaller than foo's array parameter, and will cause access to memory outside its bounds

8
cpp/ql/src/Likely Bugs/Conversion/ConversionChangesSign.cpp
View File

@@ -1,4 +1,4 @@
//sz is a signed integer, but malloc expects one that is unsigned.
//Negative values will be interpreted as a large number, which may
//lead to unexpected behavior
char *buf = malloc(sz);
//sz is a signed integer, but malloc expects one that is unsigned.
//Negative values will be interpreted as a large number, which may
//lead to unexpected behavior
char *buf = malloc(sz);

10
cpp/ql/src/Likely Bugs/Conversion/LossyPointerCast.cpp
View File

@@ -1,5 +1,5 @@
void f(char *p) {
int my_ptr = p; //Wrong: pointer assigned to int, would be incorrect if sizeof(char*)
//is larger than sizeof(int)
//...
}
void f(char *p) {
int my_ptr = p; //Wrong: pointer assigned to int, would be incorrect if sizeof(char*)
//is larger than sizeof(int)
//...
}

60
cpp/ql/src/Likely Bugs/InconsistentCheckReturnNull.cpp
View File

@@ -1,30 +1,30 @@
struct property {
char *name;
int value;
};
struct property * get_property(char *key);
struct property * get_property_default(char *key, int default_value);
void check_properties() {
// this call will get flagged since most
// calls to get_property handle NULL
struct property *p1 = get_property("time");
if(p1->value > 600) {
...
}
// this call will not get flagged since
// the result of the call is checked for NULL
struct property *p2 = get_property("time");
if(p2 != NULL && p2->value > 600) {
...
}
// this call will not get flagged since calls
// to get_property_default rarely handle NULL
struct property *p3 = get_property_default("time", 50);
if(p3->value > 60) {
...
}
}
struct property {
char *name;
int value;
};
struct property * get_property(char *key);
struct property * get_property_default(char *key, int default_value);
void check_properties() {
// this call will get flagged since most
// calls to get_property handle NULL
struct property *p1 = get_property("time");
if(p1->value > 600) {
...
}
// this call will not get flagged since
// the result of the call is checked for NULL
struct property *p2 = get_property("time");
if(p2 != NULL && p2->value > 600) {
...
}
// this call will not get flagged since calls
// to get_property_default rarely handle NULL
struct property *p3 = get_property_default("time", 50);
if(p3->value > 60) {
...
}
}

26
cpp/ql/src/Likely Bugs/Likely Typos/BoolValueInBitOp.cpp
View File

@@ -1,13 +1,13 @@
if (!flags & SOME_BIT) { //wrong: '!' has higher precedence than '&', so this
// is bracketed as '(!flags) & SOME_BIT', and does not
// check whether a particular bit is set.
// ...
}
if ((p != NULL) & p->f()) { //wrong: The use of '&' rather than '&&' will still
// de-reference the pointer even if it is NULL.
// ...
}
int bits = (s > 8) & 0xff; //wrong: Invalid attempt to get the 8 most significant
// bits of a short.
if (!flags & SOME_BIT) { //wrong: '!' has higher precedence than '&', so this
// is bracketed as '(!flags) & SOME_BIT', and does not
// check whether a particular bit is set.
// ...
}
if ((p != NULL) & p->f()) { //wrong: The use of '&' rather than '&&' will still
// de-reference the pointer even if it is NULL.
// ...
}
int bits = (s > 8) & 0xff; //wrong: Invalid attempt to get the 8 most significant
// bits of a short.

22
cpp/ql/src/Likely Bugs/Likely Typos/LogicalExprCouldBeSimplified.cpp
View File

@@ -1,11 +1,11 @@
if (0 || condition) { //wrong: can be converted to just 'condition'
//...
}
if (0 && condition) { //wrong: always evaluates to false, if statement can be removed
// ...
}
if ('A' == 65 && condition) { // wrong: can be converted to just 'condition'
// ...
}
if (0 || condition) { //wrong: can be converted to just 'condition'
//...
}
if (0 && condition) { //wrong: always evaluates to false, if statement can be removed
// ...
}
if ('A' == 65 && condition) { // wrong: can be converted to just 'condition'
// ...
}

60
cpp/ql/src/Likely Bugs/Likely Typos/MissingEnumCaseInSwitch.cpp
View File

@@ -1,30 +1,30 @@
typedef enum {
RED,
ORANGE,
YELLOW,
GREEN,
BLUE,
INDIGO,
VIOLET
} colors;
int f(colors c) {
switch (c) {
case RED:
//...
case GREEN:
//...
case BLUE:
//...
//wrong: does not use all enum values, and has no default
}
switch(c) {
case RED:
//...
case GREEN:
//...
default:
//correct: does not use all enum values, but has a default
}
}
typedef enum {
RED,
ORANGE,
YELLOW,
GREEN,
BLUE,
INDIGO,
VIOLET
} colors;
int f(colors c) {
switch (c) {
case RED:
//...
case GREEN:
//...
case BLUE:
//...
//wrong: does not use all enum values, and has no default
}
switch(c) {
case RED:
//...
case GREEN:
//...
default:
//correct: does not use all enum values, but has a default
}
}

24
cpp/ql/src/Likely Bugs/Memory Management/PotentialBufferOverflow.cpp
View File

@@ -1,12 +1,12 @@
void f(char* s, float f) {
char buf[30];
//wrong: gets has no limit to the length of data it puts in the buffer
gets(buf);
//wrong: sprintf does not limit the length of the string put into buf
sprintf(buf, "This is a string: %s", s);
//wrong: %f can expand to a very long string in extreme cases, easily overrunning this buffer
sprintf(buf, "This is a float: %f", f);
}
void f(char* s, float f) {
char buf[30];
//wrong: gets has no limit to the length of data it puts in the buffer
gets(buf);
//wrong: sprintf does not limit the length of the string put into buf
sprintf(buf, "This is a string: %s", s);
//wrong: %f can expand to a very long string in extreme cases, easily overrunning this buffer
sprintf(buf, "This is a float: %f", f);
}

8
cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.cpp
View File

@@ -1,4 +1,4 @@
strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest
strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest.
//Also fails if dest is a pointer and not an array.
strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest
strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest.
//Also fails if dest is a pointer and not an array.

8
cpp/ql/src/Likely Bugs/Memory Management/SuspiciousSizeof.cpp
View File

@@ -1,4 +1,4 @@
void f(char s[]) {
int size = sizeof(s); //wrong: s is now a char*, not an array.
//sizeof(s) will evaluate to sizeof(char *)
}
void f(char s[]) {
int size = sizeof(s); //wrong: s is now a char*, not an array.
//sizeof(s) will evaluate to sizeof(char *)
}

32
cpp/ql/src/Likely Bugs/NestedLoopSameVar.cpp
View File

@@ -1,16 +1,16 @@
int x1 = 0;
for (x1 = 0; x1 < 100; x1++) {
int x2 = 0;
for (x1 = 0; x1 < 300; x1++) {
// this is most likely a typo
// the outer loop will exit immediately
}
}
for (x1 = 0; x1 < 100; x1++) {
if(x1 == 10 && condition) {
for (; x1 < 75; x1++) {
// this should be written as a while loop
}
}
}
int x1 = 0;
for (x1 = 0; x1 < 100; x1++) {
int x2 = 0;
for (x1 = 0; x1 < 300; x1++) {
// this is most likely a typo
// the outer loop will exit immediately
}
}
for (x1 = 0; x1 < 100; x1++) {
if(x1 == 10 && condition) {
for (; x1 < 75; x1++) {
// this should be written as a while loop
}
}
}

62
cpp/ql/src/Likely Bugs/OO/NonVirtualDestructor.cpp
View File

@@ -1,31 +1,31 @@
class Base {
public:
Resource *p;
Base() {
p = createResource();
}
//...
~Base() {
//wrong: this destructor is non-virtual, but Base has a derived class
// with a non-virtual destructor
freeResource(p);
}
};
class Derived: public Base {
public:
Resource *dp;
Derived() {
dp = createResource2();
}
~Derived() {
freeResource2(dp);
}
};
int f() {
Base *b = new Derived(); //creates resources for both Base::p and Derived::dp
//...
delete b; //will only call Base::~Base(), leaking the resource dp.
// Change both destructors to virtual to ensure they are both called.
}
class Base {
public:
Resource *p;
Base() {
p = createResource();
}
//...
~Base() {
//wrong: this destructor is non-virtual, but Base has a derived class
// with a non-virtual destructor
freeResource(p);
}
};
class Derived: public Base {
public:
Resource *dp;
Derived() {
dp = createResource2();
}
~Derived() {
freeResource2(dp);
}
};
int f() {
Base *b = new Derived(); //creates resources for both Base::p and Derived::dp
//...
delete b; //will only call Base::~Base(), leaking the resource dp.
// Change both destructors to virtual to ensure they are both called.
}

38
cpp/ql/src/Likely Bugs/OO/ThrowInDestructor.cpp
View File

@@ -1,19 +1,19 @@
class C {
public:
//...
~C(){
if (error) {
throw "Exception in destructor"; //wrong: exception thrown in destructor
}
}
};
void f() {
C* c = new C();
try {
doOperation(c);
delete c;
} catch ( char * do_operation_exception) {
delete c; //would immediately terminate program if C::~C throws an exception
}
}
class C {
public:
//...
~C(){
if (error) {
throw "Exception in destructor"; //wrong: exception thrown in destructor
}
}
};
void f() {
C* c = new C();
try {
doOperation(c);
delete c;
} catch ( char * do_operation_exception) {
delete c; //would immediately terminate program if C::~C throws an exception
}
}

26
cpp/ql/src/Likely Bugs/ShortLoopVarName.cpp
View File

@@ -1,14 +1,14 @@
int i = 0;
for (i = 0; i < NUM_RECORDS; i++) {
int j = 0;
//This loop should have a more descriptive iteration variable
for (j = 0; j < NUM_FIELDS; j++) {
process(record[i]->field[j]);
}
int field_idx = 0;
//Better: the inner loop has a descriptive name
for (field_idx = 0; field_idx < NUM_FIELDS; field_idx++) {
save(record[i]->field[field_idx]);
}
int i = 0;
for (i = 0; i < NUM_RECORDS; i++) {
int j = 0;
//This loop should have a more descriptive iteration variable
for (j = 0; j < NUM_FIELDS; j++) {
process(record[i]->field[j]);
}
int field_idx = 0;
//Better: the inner loop has a descriptive name
for (field_idx = 0; field_idx < NUM_FIELDS; field_idx++) {
save(record[i]->field[field_idx]);
}
}

56
cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.c
View File

@@ -1,28 +1,28 @@
void main(int argc, char **argv) {
uint32_t big_num = INT32_MAX;
char buf[big_num];
int16_t bytes_received = 0;
int max_get = INT16_MAX + 1;
// BAD: 'bytes_received' is compared with a value of a wider type.
// 'bytes_received' overflows before reaching 'max_get',
// causing an infinite loop
while (bytes_received < max_get)
bytes_received += get_from_input(buf, bytes_received);
}
uint32_t bytes_received = 0;
// GOOD: 'bytes_received2' has a type at least as wide as 'max_get'
while (bytes_received < max_get) {
bytes_received += get_from_input(buf, bytes_received);
}
}
int getFromInput(char *buf, short pos) {
// write to buf
// ...
return 1;
}
void main(int argc, char **argv) {
uint32_t big_num = INT32_MAX;
char buf[big_num];
int16_t bytes_received = 0;
int max_get = INT16_MAX + 1;
// BAD: 'bytes_received' is compared with a value of a wider type.
// 'bytes_received' overflows before reaching 'max_get',
// causing an infinite loop
while (bytes_received < max_get)
bytes_received += get_from_input(buf, bytes_received);
}
uint32_t bytes_received = 0;
// GOOD: 'bytes_received2' has a type at least as wide as 'max_get'
while (bytes_received < max_get) {
bytes_received += get_from_input(buf, bytes_received);
}
}
int getFromInput(char *buf, short pos) {
// write to buf
// ...
return 1;
}

40
cpp/ql/src/jsf/4.10 Classes/AV Rule 95.cpp
View File

@@ -1,20 +1,20 @@
enum Shape_color { red, green, blue };
class Shape
{
public:
virtual void draw (Shape_color color = green) const;
...
}
class Circle : public Shape
{
public:
virtual void draw (Shape_color color = red) const;
...
}
void fun()
{
Shape* sp;
sp = new Circle;
sp->draw (); // Invokes Circle::draw(green) even though the default
} // parameter for Circle is red.
enum Shape_color { red, green, blue };
class Shape
{
public:
virtual void draw (Shape_color color = green) const;
...
}
class Circle : public Shape
{
public:
virtual void draw (Shape_color color = red) const;
...
}
void fun()
{
Shape* sp;
sp = new Circle;
sp->draw (); // Invokes Circle::draw(green) even though the default
} // parameter for Circle is red.