9
\$\begingroup\$

If I have a oneliner in C such as

size_t maxSizeOf(const char *s)
{
     return strlen(s) > M_MAX_SIZE ? M_MAX_SIZE : strlen(s);
}

would it be better to have a macro defined such that

#define MAX_SIZE_OF(s) (strlen(s) > M_MAX_SIZE ? M_MAX_SIZE : strlen(s))

or would it be better to have

size_t maxSizeOf(const char *s)
{
     size_t len = strlen(s);
     return len > M_MAX_SIZE ? M_MAX_SIZE : len;
}

Also, is there a way to turn the second function into a macro for maximum efficiency?

\$\endgroup\$
20
\$\begingroup\$

All three versions can be improved.

The first function has the problem that strlen(s) is likely to be called twice. strlen() is an expensive operation, as it walks along the entire string until it finds the NUL terminator.


The second version (the macro) has the same issue, with the additional caveat that it breaks if s is an expression with a side-effect, such as an assignment. For example, MAX_SIZE_OF(str++) would expand to

(strlen(str++) > M_MAX_SIZE ? M_MAX_SIZE : strlen(str++))

and possibly lead to str being incremented twice. (Even if you decide that the ALL_CAPS_NAME alerts programmers to the fact it is a macro and thus susceptible to this kind of thing, it's still a trap that is better avoided altogether.)


The last version is better, in that it avoids both the second call to strlen() and the double-argument-expansion problem. However, I still have two objections.

First, I think that you would be better off decomposing the problem into two parts, like MIN(M_MAX_SIZE, strlen(s)). The MIN() macro, although not in the C standard, is frequently reimplemented, and widely understood. On the other hand, it is less obvious what your maxSizeOf() is supposed to do. I'm not sure that it's even worth the brain power to keep track of the existence of this function.

Second, if the string that is passed to the function is much longer than M_MAX_SIZE, it still needs to walk all the way to the end of the string — even if M_MAX_SIZE is 5 and the string is a gigabyte long. To avoid that problem, I think that you may be better off walking down s yourself. And if you do hunt for the NUL terminator yourself, then it is suddenly worthwhile to have this function again.

More importantly, worrying about the efficiency of the function compared to the macro is a false optimization. The operation that could potentially take a long time and scale poorly is strlen(). That's the performance problem that would bite you — not the function-vs.-macro stuff that you are worrying about.

\$\endgroup\$
  • \$\begingroup\$ David Foerster points out that there is a strnlen() function. which may be even better than writing this code at all. \$\endgroup\$ – 200_success Jan 2 '16 at 16:17
17
\$\begingroup\$

Beyond the already mentioned[1] disadvantages[2], your code takes an avoidable performance hit if strlen(s)M_MAX_SIZE, since strlen() needs to count all bytes even if they exceed the limit. I'm going to outline a way to avoid this performance hit below.

POSIX.1-2008 has strnlen(3) (available in glibc v2.10+; Microsoft's CRT has an equivalent strnlen() function since at least VS2005):

Name

strnlen – determine the length of a fixed-size string

Synopsis

#include <string.h>

size_t strnlen(const char *s, size_t maxlen);

Description

The strnlen() function returns the number of bytes in the string pointed to by s, excluding the terminating null byte ('\0'), but at most maxlen. In doing this, strnlen() looks only at the first maxlen bytes at s and never beyond s+maxlen.

Return Value

The strnlen() function returns strlen(s), if that is less than maxlen, or maxlen if there is no null byte ('\0') among the first maxlen bytes pointed to by s.

So you could use the expression

strnlen(s, M_MAX_SIZE)

either directly, through a macro, or through an (inline) function. In the case of a macro this has the advantage of only evaluating each expression once.

If your runtime libraries lack strnlen, you can re-implement it trivially without the performance hit noted above using the widely available memchr(3):

size_t strnlen(const char *s, size_t maxlen) {
    const char *r = memchr(s, '\0', maxlen);
    return r ? (size_t)(r - s) : maxlen;
}

As a bonus, either way avoids buffer overruns that may occur with strlen when s points to an unterminated string or an invalid memory location.

\$\endgroup\$
11
\$\begingroup\$

The function is much better. It is in particular safe while your macro isn't. (Assuming s is never null.)

Imagine you have this in a loop somewhere:

tally += maxSizeOf(thing[++i]);

Your function will do the right thing. Your macro will potentially evaluate thing[++i] twice, incrementing i twice - and the "caller" of your macro has no idea that this could ever happen.

You can transform your function to a macro with a do { /* ... */ } while (0) trick. But don't do it. The function is exactly what you need and what you should be doing.

As for performance: let the compiler do its job. In general, your compiler will make better overall inlining decisions and other such low-level tricks than you will.

If you profile your application and you find out that that function is responsible for a measurable chunk of your runtime, then investigate what you could do to make it better. (Sometimes just making sure the compiler sees the definition (not just the declaration) of the function will make a difference.)

\$\endgroup\$
  • \$\begingroup\$ The OPs macro would evaluate thing[++i] twice in one statement. The result of that is undefined behaviour, due to modifying i twice. \$\endgroup\$ – Peter Jan 2 '16 at 7:45
  • \$\begingroup\$ @Peter: no, the ?: ternary introduces sequence points. \$\endgroup\$ – Mat Jan 2 '16 at 7:45
8
\$\begingroup\$

The inline keyword has been a keyword in C since C99. Use it! Inline functions are a much better option than macros. Macros are beasts from the previous millennium. Let them rest in peace. The function approach is much better than the macro.

Editorial comment:
It is now 2016. If your compiler is not compliant with C99 (preferably C11), it is time to get a new compiler.

\$\endgroup\$
  • 3
    \$\begingroup\$ A good optimizing compiler will often inline even without the hint. \$\endgroup\$ – keshlam Jan 1 '16 at 8:58
  • \$\begingroup\$ Is C like C++ in the sense that, conversely, a compiler may choose not to inline a function that the user has hinted via the keyword? (Barring compiler-specific attributes to force this, which I know exist.) \$\endgroup\$ – underscore_d Jan 1 '16 at 11:13
  • \$\begingroup\$ If I recall correctly, sufficiently new GCC versions ignore the inline keyword for the purpose of optimization and figure out on their own, whether to inline or not (barring the use of a special, non-default compiler option). \$\endgroup\$ – David Foerster Jan 1 '16 at 16:16
  • \$\begingroup\$ The inline keyword no longer means that the compiler must expand the function in line. It instead marks the function as an inline function. Defining an inline function in multiple compilation units is specifically allowed in the 2011 versions of both the C and C++ standards. It's best if all of those definitions are equivalent because the compiler may or may not expand the function in line, and if it doesn't, the linker can choose any one of the definitions of an inline function as the definition of that function. \$\endgroup\$ – David Hammen Jan 1 '16 at 18:24
  • \$\begingroup\$ The inline keyword NEVER meant that the compiler must expand the function inline. Even in the prestandard (K&R) days, it was a hint that the compiler has been free to ignore. None of the standards have changed that. A number of modern compilers do a better job of optimisation than most programmers, so ignore inline completely - and choose, based on their own criteria, whether to inline a function. \$\endgroup\$ – Peter Jan 2 '16 at 7:40
4
\$\begingroup\$

Since it hasn't been mentioned yet: The third version with the function and the local variable to store the length is best since it will only iterate over the string once. Of course this could be achieved via a macro as well but others have already explained why the function is the better choice

\$\endgroup\$
-5
\$\begingroup\$

I thought macro one is good because it's execute one time and inject the value everytime what we pass it's fast as compare to other , so better to go with macro

\$\endgroup\$
  • 5
    \$\begingroup\$ The macro doesn't "execute one time", the code is injected wherever the macro appears and executed every time. \$\endgroup\$ – Mat Jan 1 '16 at 7:33
  • \$\begingroup\$ (A) the macro has runtime parameters, so no, it can't just be "execute[d] one time". (B) even if it could, you're implying the compiler can't intelligently inline either type of expression (one-off or parameterised) when coded as a function. The fact that so many people with mistaken intuition think they know better than compilers is the main reason Donald Knuth is quoted so often on this subject. \$\endgroup\$ – underscore_d Jan 1 '16 at 11:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.