Code Review

During past few weeks, I've been reviewing an old codebase. Some functions were in place since 2008. You may think that those functions are bug-free. After seven years of usage, every issue should have emerged.

But the story is different. The number of errors I've found is impressive. Memory leaks, files left open, checks on conditions that can never be true (see also the last Horror Code), and, worst of all, logical errors. What do I mean with logical errors? Let me give you an example.

A Logical Error

There is a file descriptor declared as a global variable (OK, this could be considered a logical error too, but please keep reading) and a function that does some elaborations and then writes the result in a file descriptor. Among the parameters of this function there is a file descriptor... unfortunately it is never used. The writing is done on the global variable.

Everything works fine just because the global fd is the only one used in that program. What if in the future someone would have used that function passing a different fd? How long it would have take to find the bug?

By the way, the compiler has signaled with a warning that a parameter of the function was not used but nobody cared. You should always take care of warnings!

Conclusions

A code review is always a good thing to do. Probably you won't find big bugs but surely the stability and the quality of your software will be improved. And maybe that strange situation so difficult to reproduce will never be reported again.

Image by Randall Munroe licensed under a Creative Commons Attribution-NonCommercial 2.5 License.

The Day That Never Comes

The deadline is close. The customer is waiting for your fix. Your mate needs your patch before going home. No matter which of the above situations applies: the only way to accomplish your job is taking shortcuts and cutting corners.

You do not check some error conditions, use a fixed string instead of a localized one, do not properly free all allocated memory, etc. Your code compiles and seems to work fine but you know that it must be improved as soon as possible. So you tell your boss and/or the product manager. The response they usually give me is: "As soon as there will be some time, we'll fix it."

Guess what? That time never comes. There is always something more important or urgent to do, until a customer (usually an important one) reports an issue with the corners you have cut. Now the priority is to fix the problem as soon as possible, not to review the code to make sure it cannot happen again.

There is a logic in this: the customer doesn't care about code quality (even if he should). He just wants his software to work without errors. But for your company, it must be different. Why isn't it so?

Well, the answer I found is that for a customer is more important to have a quick solution than a bug-free software. It may seem pretty odd but just think about yourself. You buy a new smartphone and it just works as expected: you probably don't spam every social network to tell the world that your new iSomething is OK.

But I bet that if you find an issue and the customer service is really kind with you and the problem is solved in a couple of days, you'll tell your experience and suggest that brand to your friends.

This is called marketing and, on the past, there has been a PC producer that used to take advantage of this mechanism. But this is another story. At present, the only thing I can suggest you is to avoid shortcuts. At least unless you are in the marketing department.

Image by Nic McPhee licensed under Creative Commons Attribution-ShareAlike 2.0 Generic.

Horror Code - Copying & Pasting Errors

The following piece of code contains a trivial error.
        int v;
        char buf[11], *p;
        memset(buf, 0, sizeof(buf));

        /* Fill buf with a string */

        v = strtoul(buf, &p, 16);
        p = NULL;      // <--- This is wrong!!
        if (buf == p) {
                fprintf(stderr, "format error <%s>\n", buf);
                return -1;
        }

        /* Some other code */

        memset(buf, 0, sizeof(buf));

        /* Fill buf with a string */

        v = strtoul(buf, &p, 16);
        p = NULL;      // <--- This is wrong!!
        if (buf == p) {
                fprintf(stderr, "format error <%s>\n", buf);
                return -1;
        }
The content inside the if's will never be executed, because buf is statically allocated hence it will never be NULL. Of course the issue is the row "p = NULL;". Inside p there should be the pointer to the first invalid character (i.e. not an hexadecimal digit) inside buf. So the condition should evaluate to true if the string does not contain any valid hex number.

Kopimizm
This is a trivial error, but the thing that makes it funny is that it has been repeated twice in few rows, probably because of an unfortunate copy & paste operation.

I'm a big fan of the Ctrl+C Ctrl+V sequence, but I know that it can make you lose more time than what you save by not retyping the same thing.

In this particular case, the error is quite subtle and it can remain unnoticed for years (as actually it did). This is because strtoul() returns zero whenever the string doesn't start with an hexadecimal digit. But zero may be a valid value for further operations.

My suggestion is to always check twice when you copy and paste some code: you can easily replicate unnoticed errors.

Cookies And The Law

On June the 2nd, a new Italian law about cookies took effect. Basically it imposes:
  • to notify the user about the usage of so called technical cookies, and
  • to ask the permission to use profiling cookies (and blocking them until this permission is accorded).
For bloggers (like me) whom do not own the platform where their contents are published, this is quite troublesome since I don't think I'm able to stop anything that is delivered by Google (that is the kind provider of this virtual place).

By the way, I think that, if someone is concerned about privacy violations made through cookies, he can simply disable them in his browser. Here there are the instructions for the most common browsers:
By the way, cookies are just one among many ways to track users. Below there is a small (and incomplete) list.

Local Storage

This feature, introduced by HTML5, allows a website to save some data on your PC to be recalled later. If now you are thinking that this is what cookies are for, you are right. There is only one small difference: cookies are meant to be used by the server, while local storage is managed client side only.

But this is not a big protection, since with JavaScript is quite easy to transfer local storage information to the server.

Flash Cookies

Videos, apps and animations based on this old technology are gradually disappearing, however Adobe Flash is still widely used and its cache can be used to store information. So even if regular cookies are deleted, they can be recreated from this cache.

You can protect yourself with this Firefox add on that deletes Flash cookies when you close the browser (I don't know if there is something similar for other browsers).

Images

No, I'm not joking. Images generated on the fly by the server and the usage of the entity tag (ETag) to deal with cache invalidation can be used as a extremely persistent cookie.

The main defense you can adopt against this threat is to always use private mode navigation (or incognito mode) in your browser.

Evercookie

There is also a proof of concept that uses all the above methods and many other in order to create an immortal cookie. More information about evercookie at this link.

Your Browser

Every browser provides some information to the servers it connects to, like the underlying operating system, the screen resolution, the number of installed addons, the system fonts, etc. All these data can be used to create a fingerprint of your browser that is suitable for tracking.

You can learn more in this article on the EFF website.

Conclusions

Blocking regular cookies is just a resolution that doesn't fix the problem of being tracked in our habits. This is a more general problem that a law enforced by a single nation cannot solve.