Showing posts with label crappy code. Show all posts
Showing posts with label crappy code. Show all posts

Weird arrays in C

Recently I've saw quite strange way to create an array in C. I will describe it bellow - looks quite interesting!

Let's assume we have an array given bellow:

int myArray[4];
We could rework it, so that each element is declared separately:
    int myArray0;
    int myArray1;
    int myArray2;
    int myArray3;

Now we can obtain pointer to first element and last element, and iterate through elements in between it as with regular array. We exploit that compiler will probably put those variables in the same order in the same place in memory.

A full example is given below:

#include <stdio.h>

void processElement(int e)
{
    printf("processing element in array, it has value %d\n", e);
}

int main()
{
    int myArray0 = 0;
    int myArray1 = 1;
    int myArray2 = 2;
    int myArray3 = 3;

    // dummy way to force compiler not to optimalize-out our array members
    printf("%d\n%d\n%d\n%d\n", &myArray0, &myArray1, &myArray2, &myArray3);

    int *start = &myArray0;
    int *stop = &myArray3;

    if(start > stop)
    {
        start = &myArray3;
        stop = &myArray0;
    }

    for(int* it= start; it <= stop; *it++)
    {
       processElement(*it);
    }
}

:)

Dangling else problem

What is a dangling else problem? It's presented below - what will print this program?

#include <iostream>

int main()
{
    bool a = true;
    bool b = false;
    
    if(a)
        if (b)
            std::cout << "foo" << std::endl;
    else
        std::cout << "bar" << std::endl;

   return 0;
}

It may seem that it won't print anything, but isn't true - on the screen we will see "bar". That's because the "else" is bound to the last "if" statement that wasn't already bound to other "else".

This may lead to bugs and confusion. One easy way is to always embrace body of the "if" statement in brackets. If we want to bind the "else" to the first "if" then correct parenthesis should be added as below:

    if(a)
    {
        if (b)
        {
            std::cout << "foo" << std::endl;
        }
    }
    else
    {
        std::cout << "bar" << std::endl;
    }

If we want to bound it to the second "if", it should be:

    if(a)
    {
        if (b)
        {
            std::cout << "foo" << std::endl;
        }
        else
        {
            std::cout << "bar" << std::endl;
        }
    }

What is good is that compilers will usually produce a warning in places where this error may exist:

 In function 'int main()':
8:7: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]

Interesting way to check statuses returned by functions

If there is a set of bool functions, then sometimes below trick may be used to make the code that checks those values shorter:

bool status = true;
   
status &= foo_1();
status &= foo_2();
status &= foo_1();

printf(status ? "status: OK\n" : "status: NOK\n");

Mistakes during checking if a returned value is null

This is a bit modified real word example, found by tool for static analysis. In this code, foo() is a function that returns pointer to std::string or null pointer.

std::cout << foo()->length() << std::endl;

There's obvious bug here: executing length() method on value returned by foo() without checking if this variable is not null. At first it looks that it could be fixed in below way:

if (foo()) {
    std::cout << foo()->length() << std::endl;
}

Unfortunately above code is also invalid because it can't be sure that if a function once returned not null value, then it will returns it next time. This may be fixed in below way, where temporary variable was used to check if length() may be executed on variable retured by foo().

std::string* fooObj = foo();
if (fooObj) {
    std::cout << fooObj->length() << std::endl;
}

Access to object's fields by using pointer arithmetic

Normally a method uses its object's field by using their name. It's presented below, where we want to get value of description field from getDescription method.

class Car {
    std::string getDescription() {
        return description; // <- we return value of this field
    }

    std::string description;
};

The same result can be obtained by using a bit of magic with this, pointers and casting:

#include <iostream>

class Car {
public:
    Car (int _weight, int _maxSpeed, std::string _description) :
        weight(_weight), maxSpeed(_maxSpeed), description(_description) {
    }
    std::string getDescription() {
        return description;
    }
    std::string getDescriptionUsingThisKeyword() {
        return *((std::string*)((int*)this + 2));
    } 
private:
    int weight, maxSpeed;
    std::string description;
};

int main() {
    Car myCar(200, 200, "my best car ever");
    std::cout << myCar.getDescription() << std::endl;
    std::cout << myCar.getDescriptionUsingThisKeyword() << std::endl;
}

Output:

bash-3.2$ ./a.out 
my best car ever
my best car ever

It's error-prone, e.g. when someone changes order of fields in the class. I think, that it's cool but I wouldn't like to see this in a real code :)

Less popular way to copy content of a structure

There is instance (let's call it p0) of structure, we create new instance (p1) of the same structure and want to copy content of fields from p0 to p1.

The simplest (IMHO also the best) way is to copy field by field. It's presented in following code (in case of p1 all fields are copied, in case of p2 only some).

/* output: 
foo (3.800000, 4.900000): #121212
bar (3.800000, 4.900000): #335566
*/
#include <stdlib.h>

struct Point {
    char desc[5];
    double x, y;
    char color[8];
};

int main() {
    struct Point p0 = {.desc = "foo", .x = 3.8, .y = 4.9, .color = "#121212"};

    struct Point p1;
    snprintf(p1.desc, sizeof(p1.desc), p0.desc);
    p1.x = p0.x;
    p1.y = p0.y;
    snprintf(p1.color, sizeof(p1.color), p0.color);
    printf("%s (%f, %f): %s\n", p1.desc, p1.x, p1.y, p1.color);

    struct Point p2;
    snprintf(p2.desc, sizeof(p2.desc), "bar");
    p2.x = p0.x;
    p2.y = p0.y;
    snprintf(p2.color, sizeof(p2.color), "#335566");
    printf("%s (%f, %f): %s\n", p2.desc, p2.x, p2.y, p2.color);

    return EXIT_SUCCESS;
}

The second way is use memcpy to copy content of memory allocated for p0 directly into p1. It's presented in next snippet.