The Stack Overflow podcast is back! Listen to an interview with our new CEO.

Hot answers tagged

26

Although the C standard can allow void main() under certain implementations, it's best to use int main() whenever possible. See this for more info on the return type of main(). A loop is not a good use of a macro: #define printS for(i=0;s[i];i++){printf("%c",s[i]);} It should instead be a function: void printS() { for (i = 0; s[i]; i++) { ...


20

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, ...


18

Apart from the usage of a macro where a function would have been sufficient (don't use macros if there is another language construct that can do the job!) and the global variable (never do this unless it is absolutely needed), there is one big thing that has not yet been mentioned: Why do you print the string by calling printf() for every single character? ...


18

p = malloc(n*sizeof *p); This is dangerous if n gets large, because the multiplication could overflow. After the overflow, too little memory has been allocated but code will continue without detecting the error. This is especially dangerous if n comes from untrusted source, such as some file format or remote user. Then it gives attacker easy way to ...


17

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() ...


17

First off, Jean-François is absolutely right: you cannot assume any particular bit widths for the built-in types, short, int, long, etc. Use the types defined in stdint.h that have explicit bit widths to ensure that the code is correct and portable. Otherwise, your code looks pretty good, and this is a reasonable implementation. But… Is this a ...


16

It is not thread safe. You can use Boost, or standard C++11 std::call_once. Answer to comment: Boost and C++11 are defining include library for launching threads and thread synchronization (locks, atomic variables…). The call_once function can either use those to ensure thread safety or use the thread lib of the OS (pthreads for *nix). A very simplified (...


16

Efficiency The main issue with std::function<void()> is that it is heavy in memory usage. It comes in at a minimum of 32 bytes in G++ and (if memory serves me right) 40 bytes in VC++2015 32-bit. See: http://coliru.stacked-crooked.com/a/9503ca27f1faded8 Solution Don't use std::function<void()>. A lightweight implementation I promise you that ...


13

There is a rather large amount of code here and a lot of macro trickery, so I shall refrain from attempting to comment on it all. There are, however, a few things that immediately jumped out at me. Don't use new as your allocating macro. That's extremely confusing, and it's going to make compiling it as C++ rather difficult. (Not that having C be ...


12

Note that macros such as this should be placed in a do { ... } while (0) block, otherwise you will encounter problems when the macro is invoked prior to an else, i.e. #define RUN_ONCE(runcode) \ do \ { \ static bool code_ran = 0; \ if (!code_ran) \ { \ code_ran = 1; \ runcode; \ } \ } while (0) e.g. if you have a situation ...


12

If you are using GCC, we can use the typeof()(C99) keyword to get rid of one of the arguments. Also, add a do-while so the macro to be used in contexts where it would otherwise be problematic. #define SWAP(a, b) do { typeof(a) t; t = a; a = b; b = t; } while(0) You could also use an exclusive-or (^=) to get rid of that temporary variable, but that only ...


11

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 ...


10

First, macros should have ALL_CAPS names to distinguish them from functions. Users need to be able to distinguish macros from functions because they behave slightly differently. For example, with your definition of swap() as a macro, this code (for illustration purposes — not that it's good code) fails to compile: if (swap(int, x, y), x) { /* Do ...


9

Just a few things to point out: There's no need to use inline yourself. For modern compilers, it merely serves as a suggestion, but they can otherwise determine if and when it's really needed. Read this for more info. You should use consistent naming for your namespaces (one is lowercase and the other is uppercase). I'd not use uppercase as it's commonly ...


9

Here's what I came up with. I'm not sure if this is better or worse, but it's less code. Feel free to review this instead of the code in the question (or both). Some extra definitions get around the upper/lower case problem mentioned in the question: #define LUA_Tlightuserdata LUA_TLIGHTUSERDATA #define LUA_Tstring LUA_TSTRING #define LUA_Tnumber ...


9

If you need to check if a certain method exists you can also check if the object supports it via respondsToSelector: like this: if ([object respondsToSelector:@selector(openURL:options:completionHandler:)]) { [object openURL:...options:...completionHandler:...]; } else { [object openURL:...]; } I think this approach is more flexible than checking ...


8

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 ...


8

If you have a C++ standards-compliant compiler, then you should not need to use <conio.h> under Windows (_kbhit and _getch). std::cin will work. I know there are still old compilers floating around, but there are various modern C++ compilers available for free (GCC, Clang and MSVC), I don't think there is any excuse to code for non-standards compliant ...


8

You've taken an expression statement and unnecessarily made a do/while statement out of it. You need parentheses around your macro parameters. You don't need to pass in the pointer you're assigning to as a pointer. Put all that together and you end up with: #define ALLOC(p, n) ((p) = malloc((n) * sizeof *(p))) This puts fewer restrictions on how it is ...


7

I did manage to improve some things since the question was posted. So here is what I discovered, that could somehow improve the implementation and the usability of the Vector(T): First of all, there was an error in the code. Shame on me. I shared a piece of code that could not be compiled. I must have posted a version that was not up-to-date. The macro new ...


7

Is there a situation where the compiler will not optimize away the wrapper because the wrapper and the original function have different semantics? Yes to the first part and no to the second. Any optimizing compiler will inline your wrapper, as it consists of only one return statement. But on the other hand, the original function might have different ...


6

[ Foreword: Note that I'm not going to argue whether or not it is a good idea to invent a new macro-based syntax for this. I'm only reviewing your code. — end foreword ] I think you have been pretty careful and cannot spot any serious issues with your implementation; nice job. The only thing that I think could be improved is the error reporting. ...


6

Keep the DLLExport macro on all platforms, but make the definition resolve to nothing unless you're on Windows. You might also want to rename the macro to something more neutral. Something like this: #if defined(WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(_WIN64) || defined(WINAPI_FAMILY) # define EXPORT __declspec(dllexport) #else # define ...


6

I found the basic model of how the program works more annoying than useful. I'd expect typing just "cal" to pick a reasonable default, such as displaying the calendar for the current month (and possibly the previous and next months). I'd use the standard library for most of the calculations unless you have a specific reason to do otherwise (e.g., wanting to ...


5

I prefer this over the macro-with-arguments version. #define RUN_ONCE \ for (static bool _run_already_ = false; \ _run_already_ ? false : _run_already_ = true;) \ /***/ The above is not thread-safe, though. To make it thread safe, one could use std::atomic<>, but only if using C++11 and ...


5

You're writing a lot of code, but what is the benefit? How is this any worse? namespace MyNamespace { namespace ImplDetails { // Non-public code. } // Public code. } Yes, people can access things in ImplDetails, but you can't get around that sanely. Obscuring namespace names is only going to hurt compatibility (when a namespace gets ...


5

This question is getting old, but there is actually an elegant way to create such an exception boundary: create a function as the one proposed by @Tim Martin, then wrap the old code into lambdas that will be called by the exception boundary handle. Here it the excception boundary handle: template<typename Callable> auto exception_boundary_handle(...


5

First of all, I suggest that you use a structure instead of a hash table when you know that the table will only have 3 elements; this improves both readability and performance: (defstruct triplestore-graph (spo (make-hash-table)) (pos (make-hash-table)) (osp (make-hash-table))) (defvar *triplestore-graph* (make-triplestore-graph)) Next, there is no ...


5

Using a macro is OK in C89 which did not have inline functions. But your macro has extremely bad style and I believe it would be considered unacceptable in most, if not all, places: It messes with a static variable i It doesn't have () , yet it executes statements It doesn't play nicely with surrounding code It evaluates s more than once For example int i ...


Only top voted, non community-wiki answers of a minimum length are eligible