The knowledge you have, working with one language isn’t always applicable to another language

The fragment is taken from Putty project. Ineffective code is detected by the following diagnostic: V814 Decreased performance. Calls to the ‘strlen’ function have being made multiple times when a condition for the loop’s continuation was calculated.

static void tell_str(FILE * stream, char *str)
{
  unsigned int i;
  for (i = 0; i < strlen(str); ++i)
    tell_char(stream, str[i]);
}

Explanation

There’s no actual error here, but such code can be extremely inefficient when we deal with long strings, as the strlen() function is called in every loop iteration. So the error, if there is one here, is one of inefficiency.

As a rule, this kind of thing is typically found in code written by those that have previously worked with the Pascal language (or Delphi). In Pascal, the evaluation of the terminating condition of the loop is computed just once, thus this code is suitable and quite commonly used.

hGd_2R66SYtZZAKfQ4b898btrYEKK2ZY

Let’s have a look at an example of code written in Pascal. The word called will be printed only once, because the pstrlen() is called only once.

program test;
var
  i   : integer;
  str : string;

function pstrlen(str : string): integer;
begin
  writeln('called');
  strlen := Length(str);
end;

begin
  str := 'a pascal string';
  for i:= 1 to pstrlen(str) do 
    writeln(str[i]);
end.

Effective code:

static void tell_str(FILE * stream, char *str)
{
  size_t i;
  const size_t len = strlen(str);
  for (i = 0; i < len; ++i)
    tell_char(stream, str[i]);
}

Recommendation

Don’t forget that in C/C++, loop termination conditions are re-computed at the end of each and every iteration. Therefore it’s not a good idea to call inefficient slow functions as part of this evaluation, especially if you can compute it just the once, before the loop is entered.

In some cases the compiler might be able to optimize the code with strlen(). For instance, if the pointer always refers to the same string literal, but we shouldn’t rely on that in any way.

Written by Andrey Karpov.

This error was found with PVS-Studio static analysis tool.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s