# Code Quality Comparison of Firebird, MySQL, and PostgreSQL Today's article is somewhat unusual, if only because instead of reviewing one project, we'll be comparing three projects at once, looking for the one with the most interesting bugs and \- which is of particular interest \- the one with the highest code quality\. The projects we are going to review are Firebird, MySQL, and PostgreSQL\. So let's get started\! ![0542_Firebird_MySQL_PostgreSQL/image1.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image1.png) ## A few words about the projects ### Firebird ![0542_Firebird_MySQL_PostgreSQL/image2.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image2.png) Firebird \(FirebirdSQL\) is an open\-source SQL relational database management system that runs on Linux, Microsoft Windows, macOS X, and a variety of Unix\. The database forked from Borland's open\-source edition of InterBase in 2000, but since Firebird 1\.5 the code has been largely rewritten\. **Additional information:** * [official website](https://firebirdsql.org/); * [GitHub repository](https://github.com/FirebirdSQL/firebird); * stars at GitHub: 133; * forks at GitHub: 51\. ### MySQL ![0542_Firebird_MySQL_PostgreSQL/image3.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image3.png) MySQL is an open\-source relational database management system \(RDBMS\)\. MySQL is typically used as a server for local and remote clients, but the distribution also includes an embedded MySQL server library, which makes it possible to run a MySQL server inside a client application\. MySQL supports multiple table types, which makes it a very flexible tool: users can choose between MyISAM tables, which support full\-text search, and InnoDB tables, which support transactions at the level of individual records\. MySQL also comes with a special table type called EXAMPLE, which is used to demonstrate the principles of creating new table types\. Thanks to the open architecture and GPL\-licensing, new types are regularly added to MySQL\. **Additional information**: * [official website](https://www.mysql.com/); * [GitHub repository](https://github.com/mysql/mysql-server); * stars at GitHub: 2179; * forks at GitHub: 907\. ### PostgreSQL ![0542_Firebird_MySQL_PostgreSQL/image4.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image4.png) PostgreSQL is an object\-relational database management system \(ORDBMS\)\. It can handle workloads ranging from small single\-machine applications to large Internet\-facing applications \(or for data warehousing\) with many concurrent users; on macOS Server, PostgreSQL is the default database; and it is also available for Microsoft Windows and Linux \(supplied in most distributions\)\. PostgreSQL is developed by the PostgreSQL Global Development Group, a diverse group of many companies and individual contributors\. It is free and open\-source, released under the terms of the PostgreSQL License, a permissive software license\. **Additional information**: * [official website](https://www.postgresql.org/); * [GitHub repository mirror](https://github.com/postgres/postgres); * stars at GitHub: 3260; * forks at GitHub: 1107\. ## PVS\-Studio ![0542_Firebird_MySQL_PostgreSQL/image5.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image5.png) I was using static code analyzer [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) to detect bugs\. PVS\-Studio is an analyzer for source code written in C, C\+\+, and C\# which helps reduce software development costs due to early detection of bugs, defects, and security issues in programs' source code\. It runs on Windows and Linux\. Download links: * [for Windows](https://pvs-studio.com/en/pvs-studio/download/); * [for Linux](https://pvs-studio.com/en/pvs-studio/download/)\. Because each of the three projects is fairly easy to build and includes \.sln files \(either available right from the start or generated through CMake\), the analysis itself becomes quite a trivial task: you just need to start a check in the PVS\-Studio plugin for Visual Studio\. ## Comparison criteria Before starting our discussion, we have to decide what comparison criteria to use\. This is one of the primary concerns of this article\. ### Why "head\-on" comparison is not a good idea "Head\-on" comparison based on the number of error messages produced by the analyzer \(or rather the number of messages / number of LOC ratio\) for each project is not a good idea, even though it's the least costly way\. Why so? Take PostgreSQL project, for instance\. It triggers 611 high\-certainty\-level GA warnings, but if you filter these warnings by the code of PVS\-Studio diagnostic rule \([V547](https://pvs-studio.com/en/docs/warnings/v547/)\) and by the part of the message _ret < 0_, you'll see that there are 419 warnings\! That's too many, isn't it? It seems all these messages come from a single source such as a macro or automatically generated code\. Well, the comments in the beginning of the files, at which the warnings were issued, prove that our assumption is correct: ```cpp /* This file was generated automatically by the Snowball to ANSI C compiler */ ``` Now that you know that the code was generated automatically, you have two options: * Suppress all these warnings at the generated code as they are not interesting\. This cuts the total number of messages \(GA, Lvl1\) by as much as 69%\! * Accept that bugs in automatically generated code are still bugs and try to do something about them \(say, fix the code\-generating script\)\. In this case, the number of messages remains the same\. Another problem is errors found in third\-party components used in the projects\. Again, you have to choose between the same two options: * Pretend these bugs are no concern of yours \- but will the users agree with that? * Take on the responsibility for these bugs\. These are just a couple of examples of how you have to make a choice that may affect \(sometimes drastically\) the number of warnings to deal with\. ### An alternative way Let's agree right off to leave out messages of the 3 \(low\-certainty\) level\. These issues are not the ones that worth to be paid attention in the first place\. Sure, some of them might be interesting, but it's better to ignore them when you write articles and when you are only getting started with static analysis\. This review is not a full\-fledged comparison, as such a comparison would be too tedious for many reasons\. For one thing, it would require preliminary configuration of the analyzer for each of the projects, as well as looking through and examining hundreds of messages after the check\. It all takes too much time, while there's doubt if such an undertaking is really worth it\. Instead, I will look through the logs for each of the projects, pick the most interesting bugs, comment on them, and check the other two projects for similar issues\. There's one more thing I should mention\. We've started to pay attention to security issues lately and even posted an article titled "[How Can PVS\-Studio Help in the Detection of Vulnerabilities?](https://pvs-studio.com/en/blog/posts/cpp/0514/)" Since one of today's participants, MySQL, had been mentioned in that article, I was curious to see if PVS\-Studio would detect any of those specific code patterns\. No gimmicks \- we'll just additionally look for warnings similar to those discussed in the article above\. ![0542_Firebird_MySQL_PostgreSQL/image6.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image6.png) So, again, I'll be evaluating the code quality based on the following criteria: * First, I will scan each of the three logs for the same warnings as discussed in the above\-mentioned article on security issues\. The idea is simple: if you know that a certain code pattern could be a vulnerability \(even though not all the time\), then you should take a closer look at it\. * Then I will look through the GA warnings of the first two certainty levels, pick the most interesting ones, and check if the other projects have triggered similar warnings\. As we proceed, I'll be giving demerit points to each project, so the one with the fewest points will be the winner \(within the restrictions discussed earlier\)\. There are some specific details, of course, but I'll be commenting on these along the way and at the end of the article\. Here we go\! ## Review of bugs ### Total analysis results The table below shows the total analysis results "as is', i\.e\. with no false\-positives suppressed, without any filtering by folders, and so on\. Note that the warnings refer only to the General Analysis set\. |Project|High Certainty|Medium Certainty|Low Certainty|Total| |---|---|---|---|---| |Firebird|156|680|1045|1881| |MySQL|902|1448|2925|5275| |PostgreSQL|611|1432|1576|3619| This table, however, is a poor basis for drawing any conclusions about the code quality\. As I already said, there is a number of reasons: * no analyzer preliminary configuration; * no false\-positive suppression; * different sizes of the codebases; * we were making changes to the analyzer while working on this article, so the "before" and "after" results may be slightly different\. As for the density of warnings \(not bugs\!\), i\.e\. the ratio between the number of messages and LOC, as measured without preliminary configuration, it is roughly the same for Firebird and PostgreSQL, and is a bit higher for MySQL\. But let's not jump to conclusions because, you know, the devil is in the detail\. ### Troubles with clearing private data The [V597](https://pvs-studio.com/en/docs/warnings/v597/) diagnostic is issued by a presence of such a call of _memset_ function, performing data clearing, which can be removed by a compiler when optimization\. As a result, private data might remain uncleared\. For details, see the [documentation on the diagnostic](https://pvs-studio.com/en/docs/warnings/v597/)\. Neither Firebird, nor PostgreSQL triggered any messages of this type, but MySQL did\. So, it is MySQL that the following example is taken from: ```cpp extern "C" char * my_crypt_genhash(char *ctbuffer, size_t ctbufflen, const char *plaintext, size_t plaintext_len, const char *switchsalt, const char **params) { int salt_len; size_t i; char *salt; unsigned char A[DIGEST_LEN]; unsigned char B[DIGEST_LEN]; unsigned char DP[DIGEST_LEN]; unsigned char DS[DIGEST_LEN]; .... (void) memset(A, 0, sizeof (A)); (void) memset(B, 0, sizeof (B)); (void) memset(DP, 0, sizeof (DP)); (void) memset(DS, 0, sizeof (DS)); return (ctbuffer); } ``` **PVS\-Studio warnings**: * V597 The compiler could delete the 'memset' function call, which is used to flush 'A' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 420 * V597 The compiler could delete the 'memset' function call, which is used to flush 'B' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 421 * V597 The compiler could delete the 'memset' function call, which is used to flush 'DP' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 422 * V597 The compiler could delete the 'memset' function call, which is used to flush 'DS' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. crypt\_genhash\_impl\.cc 423 The analyzer detected a function with as many as 4 buffers \(\!\), which must be forcibly cleared\. However, the function could fail to do so, causing the data to remain in memory "as is"\. Since buffers _A_, _B_, _DP_, and _DS_ are not used later on, the compiler is allowed to remove the call to the _memset_ function because such an optimization does not affect the program's behavior from the viewpoint of the C/C\+\+ language\. For more information about this issue, see the article "[Safe Clearing of Private Data](https://pvs-studio.com/en/blog/posts/cpp/0388/)"\. The rest messages are not any different, so I'll just list them: * V597 The compiler could delete the 'memset' function call, which is used to flush 'table\_list' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sql\_show\.cc 630 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 413 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 490 * V597 The compiler could delete the 'memset' function call, which is used to flush 'T' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 491 * V597 The compiler could delete the 'memset' function call, which is used to flush 'W' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 597 * V597 The compiler could delete the 'memset' function call, which is used to flush 'T' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. sha\.cpp 598 Here's a more interesting case\. ```cpp void win32_dealloc(struct event_base *_base, void *arg) { struct win32op *win32op = arg; .... memset(win32op, 0, sizeof(win32op)); free(win32op); } ``` **PVS\-Studio warning**: [V597](https://pvs-studio.com/en/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'win32op' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. win32\.c 442 It is similar to the previous example except that after the memory block is cleared the pointer will be passed to the _free _function\. But even then the compiler is still allowed to remove the call to _memset_, leaving only the call to _free_ \(which clears the memory block\)\. As a result, the data that was to be cleared remains in memory\. For more information, see the above\-mentioned article\. **Assigning demerit points\.** This is quite a serious error \- even more so because there are three instances of it\. 3 demerit points go to MySQL\. ### No check for the pointer returned by malloc and other similar functions All the three projects triggered [V769](https://pvs-studio.com/en/docs/warnings/v769/) warnings\. * Firebird: high certainty \- 0; medium certainty \- 0; low certainty \- 9; * MySQL: high certainty \- 0; medium certainty \- 13; low certainty \- 103; * PostgreSQL: high certainty \- 1 medium certainty \- 2; low certainty \- 24\. Since we agreed to ignore third\-level warnings, we continue without Firebird \(so much the better for it\)\. All the three warnings in PostgreSQL proved irrelevant too\. This leaves only MySQL: it also triggered a few false positives, but some of the warnings are worth looking at\. ```cpp bool Gcs_message_stage_lz4::apply(Gcs_packet &packet) { .... unsigned char *new_buffer = (unsigned char*) malloc(new_capacity); unsigned char *new_payload_ptr = new_buffer + fixed_header_len + hd_len; // compress payload compressed_len= LZ4_compress_default((const char*)packet.get_payload(), (char*)new_payload_ptr, static_cast(old_payload_len), compress_bound); .... } ``` **PVS\-Studio warning**: [V769](https://pvs-studio.com/en/docs/warnings/v769/) The 'new\_buffer' pointer in the 'new\_buffer \+ fixed\_header\_len' expression could be nullptr\. In such case, resulting value will be senseless and it should not be used\. Check lines: 74, 73\. gcs\_message\_stage\_lz4\.cc 74 If it fails to allocate the requested memory block, the _malloc_ function returns a null pointer that could be stored to the _new\_buffer _variable\. Next, as the _new\_payload\_ptr_ variable is initialized, the value of the _new\_buffer_ pointer is added to the values of variables _fixed\_header\_len_ and _hd\_len_\. This is a point of no return for _new\_payload\_ptr_: if later on \(say, in another function\) we decide to check it for _NULL_, such a check won't help\. No need to tell you what the implications are\. So, it would be wiser to make sure that _new\_buffer_ is non\-null before initializing _new\_payload\_ptr_\. You may argue that since _malloc_ has failed to allocate the requested memory block, then there's not much sense in checking its return value for _NULL_ either\. The application can't continue its normal work anyway, so why not let it crash the next time it uses the pointer? Since quite a lot of developers stick to this approach, it can be called legitimate \- but is this approach right? After all, you could try to somehow handle that case to save the data or have the application crash in a "softer way"\. Besides, this approach might lead to security issues because if the application happens to handle another memory block \(_null pointer_ \+ _value_\) rather than the null pointer itself, it may well damage some data\. All this makes your program even more vulnerable\. Are you sure you want it that way? Anyway, you have to decide for yourself what the pros and cons are and which choice is right\. I recommend the second approach \- the [V769](https://pvs-studio.com/en/docs/warnings/v769/) diagnostic will help you detect those issues\. However, if you are sure that such functions can never return _NULL_, tell the analyzer about it so you don't get the same warnings again\. See the article "[Additional diagnostics configuration](https://pvs-studio.com/en/docs/manual/0040/)" to find out how\. **Assigning demerit points\.** Considering everything said above, MySQL is given 1 demerit point\. ### The use of a potential null pointer Warnings of this type \(diagnostic [V575](https://pvs-studio.com/en/docs/warnings/v575/)\) were found in each of the three projects\. This is an example from Firebird \(medium certainty\): ```cpp static void write_log(int log_action, const char* buff) { .... log_info* tmp = static_cast(malloc(sizeof(log_info))); memset(tmp, 0, sizeof(log_info)); .... } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The potential null pointer is passed into 'memset' function\. Inspect the first argument\. Check lines: 1106, 1105\. iscguard\.cpp 1106 This defect is similar to the previous one \- no check for the return value of the _malloc_ function\. If it fails to allocate the requested block of memory, _malloc_ will return a null pointer, which will then be passed to the _memset_ function\. Here is a similar example from MySQL: ```cpp Xcom_member_state::Xcom_member_state(....) { .... m_data_size= data_size; m_data= static_cast(malloc(sizeof(uchar) * m_data_size)); memcpy(m_data, data, m_data_size); .... } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The potential null pointer is passed into 'memcpy' function\. Inspect the first argument\. Check lines: 43, 42\. gcs\_xcom\_state\_exchange\.cc 43 This is similar to what we saw in Firebird\. Just to make it clear, there are some fragments of code where the returned value _malloc_ is checked for inequality to null\. The following is a similar fragment from PostgreSQL: ```cpp static void ecpg_filter(const char *sourcefile, const char *outfile) { .... n = (char *) malloc(plen); StrNCpy(n, p + 1, plen); .... } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The potential null pointer is passed into 'strncpy' function\. Inspect the first argument\. Check lines: 66, 65\. pg\_regress\_ecpg\.c 66 MySQL and PostgreSQL, however, triggered a few high\-certainty\-level warnings, which are of more interest\. An example from MySQL: ```cpp View_change_event::View_change_event(char* raw_view_id) : Binary_log_event(VIEW_CHANGE_EVENT), view_id(), seq_number(0), certification_info() { memcpy(view_id, raw_view_id, strlen(raw_view_id)); } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. control\_events\.cpp 830 The _memcpy_ function is used to copy the string from _raw\_view\_id_ to _view\_id_; the number of bytes to copy is calculated using the _strlen_ function\. The problem here is that _strlen_ ignores the terminating null character, so the string is copied without it\. If you then don't add it by hand, other string functions will not be able to handle _view\_id _properly\. To ensure correct copying of the string, use _strcpy_ / _strcpy\_s_\. Now, the following fragment from PostgreSQL looks very much the same: ```cpp static int PerformRadiusTransaction(char *server, char *secret, char *portstr, char *identifier, char *user_name, char *passwd) { .... uint8 *cryptvector; .... cryptvector = palloc(strlen(secret) + RADIUS_VECTOR_LENGTH); memcpy(cryptvector, secret, strlen(secret)); } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. auth\.c 2956 There is, however, an interesting difference from the previous example\. The _cryptvector_ variable is of type _uint8\*_\. While _uint8_ is an alias for _unsigned char_, the programmer seems to be using it to explicitly indicate that these data is not meant to be handled as a string; so, given the context, this operation is valid and is not as suspicious as the previous case\. Some of the reported fragments, however, don't look that safe\. ```cpp int intoasc(interval * i, char *str) { char *tmp; errno = 0; tmp = PGTYPESinterval_to_asc(i); if (!tmp) return -errno; memcpy(str, tmp, strlen(tmp)); free(tmp); return 0; } ``` **PVS\-Studio warning**: [V575](https://pvs-studio.com/en/docs/warnings/v575/) The 'memcpy' function doesn't copy the whole string\. Use 'strcpy / strcpy\_s' function to preserve terminal null\. informix\.c 677 This issue follows the same pattern but is more like the example from MySQL: it deals with string operations and copying of a string's contents \(except for the terminating null character\) to memory used outside the function\.\.\. **Assigning demerit points\.** 1 demerit point goes to Firebird and 3 demerit points go to PostgreSQL and MySQL each \(one point for a medium\-certainty warning, two points for a high\-certainty one\)\. ### Potentially unsafe use of formatted\-output functions Only Firebird triggered a few [V618](https://pvs-studio.com/en/docs/warnings/v618/) warnings\. Take a look at this example: ```cpp static const char* const USAGE_COMP = " USAGE IS COMP"; static void gen_based( const act* action) { .... fprintf(gpreGlob.out_file, USAGE_COMP); .... } ``` **PVS\-Studio warning**: [V618](https://pvs-studio.com/en/docs/warnings/v618/) It's dangerous to call the 'fprintf' function in such a manner, as the line being passed could contain format specification\. The example of the safe code: printf\("%s", str\); cob\.cpp 1020 What alerted the analyzer is the fact that formatted\-output function _fprintf_ is used, while the string is writtendirectly, without using the format string and related specifiers\. This may be dangerous and even cause a security issue \(see [CVE\-2013\-4258](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-4258)\) if the input string happens to contain format specifiers\. In this case, though, the _USAGE\_COMP_ string is explicitly defined in the source code and doesn't include any format specifiers, so _fprintf_ can be used safely here\. The same applies to the rest cases: the input strings are hard\-coded and have no format specifiers\. **Assigning demerit points\.** Considering all said above, I'm not giving any demerit points to Firebird\. ### Other warnings mentioned in the article on vulnerabilities None of the projects triggered any [V642](https://pvs-studio.com/en/docs/warnings/v642/) and [V640](https://pvs-studio.com/en/docs/warnings/v640/) warnings \- they all did well\. ### Suspicious use of enumeration elements An example from MySQL: ```cpp enum wkbType { wkb_invalid_type= 0, wkb_first= 1, wkb_point= 1, wkb_linestring= 2, wkb_polygon= 3, wkb_multipoint= 4, wkb_multilinestring= 5, wkb_multipolygon= 6, wkb_geometrycollection= 7, wkb_polygon_inner_rings= 31, wkb_last=31 }; bool append_geometry(....) { .... if (header.wkb_type == Geometry::wkb_multipoint) .... else if (header.wkb_type == Geometry::wkb_multipolygon) .... else if (Geometry::wkb_multilinestring) .... else DBUG_ASSERT(false); .... } ``` **PVS\-Studio warning**: [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'wkb\_multilinestring' is used as a variable of a Boolean\-type\. item\_geofunc\.cc 1887 The message actually says it all\. Two of the conditional expressions compare _header\.wkb\_type_ with the elements of the _Geomerty _enumeration, while the entire third expression is itself an enumerator\. Since _Geometry::wkb\_multilinestring_ has the value _5_, the body of the third conditional statement will execute every time the previous two checks fail\. Therefore, the _else_\-branch, containing the call to the _DBUG\_ASSERT _macro_,_ will never be executed\. This suggests that the third conditional expression was meant to look like this: ```cpp header.wkb_type == Geometry::wkb_multilinestring ``` What about the rest? PostgreSQL didn't trigger any warnings of this type, while Firebird triggered as many as 9\. Those, however, are all one level less critical \(medium certainty\), and the detected pattern is different too\. The [V768](https://pvs-studio.com/en/docs/warnings/v768/) diagnostic detects the following bug patterns: * High certainty: enumeration members are used as Boolean expressions\. * Medium certainty: variables of enumeration type are used as Boolean expressions\. While there is no excuse for first\-level warnings, second\-level ones leave room for debate\. For example, this is what most cases look like: ```cpp enum att_type { att_end = 0, .... }; void fix_exception(...., att_type& failed_attrib, ....) { .... if (!failed_attrib) .... } ``` **PVS\-Studio warning**: [V768](https://pvs-studio.com/en/docs/warnings/v768/) The variable 'failed\_attrib' is of enum type\. It is odd that it is used as a variable of a Boolean\-type\. restore\.cpp 8580 The analyzer finds it suspicious that the _failed\_attrib_ variable is checked for the value _att\_type::att\_end_ in a way like that\. If you ask me, I'd prefer an explicit comparison with the enumerator, yet I can't call this code incorrect\. True, I don't like this style \(and neither does the analyzer\), but it's still legitimate\. However, two fragments look more suspecious\. Both have the same pattern, so we'll discuss only one of them\. ```cpp namespace EDS { .... enum TraScope {traAutonomous = 1, traCommon, traTwoPhase}; .... } class ExecStatementNode : .... { .... EDS::TraScope traScope; .... }; void ExecStatementNode::genBlr(DsqlCompilerScratch* dsqlScratch) { .... if (traScope) .... .... } ``` **PVS\-Studio warning**: [V768](https://pvs-studio.com/en/docs/warnings/v768/) The variable 'traScope' is of enum type\. It is odd that it is used as a variable of a Boolean\-type\. stmtnodes\.cpp 3448 This example is similar to the previous one: the programmer is also checking that the value of the _traScope_ variable is the same as the non\-zero value of the enumerator member\. However, unlike the previous example, there are no enumerator members with the value '0' here, which makes this code more suspicious\. Now that we've started talking about medium\-certainty warnings, I should add that 10 such messages were issued for MySQL as well\. **Assigning demerit points\.** Firebird is given 1 demerit point and MySQL is given 2 points\. ### Incorrect determination of memory\-block size Now, here's another interesting fragment of code\. Note that we already saw it when discussing the problem with the clearing of private data\. ```cpp struct win32op { int fd_setsz; struct win_fd_set *readset_in; struct win_fd_set *writeset_in; struct win_fd_set *readset_out; struct win_fd_set *writeset_out; struct win_fd_set *exset_out; RB_HEAD(event_map, event_entry) event_root; unsigned signals_are_broken : 1; }; void win32_dealloc(struct event_base *_base, void *arg) { struct win32op *win32op = arg; .... memset(win32op, 0, sizeof(win32op)); free(win32op); } ``` **PVS\-Studio warning**: [V579](https://pvs-studio.com/en/docs/warnings/v579/) The memset function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the third argument\. win32\.c 442 Note the third argument in the call to the _memset_ function\. The _sizeof_ operator returns the size of its argument in bytes, but here its argument is a pointer, so it returns the size of the pointer rather than the size of the structure\. This will result in incomplete memory clearing even if the compiler won't throw away the call to _memset_\. The moral is that you should choose variables' names carefully and try to avoid using similarly looking names\. It's not always possible, so pay special attention to such cases\. A lot of errors detected by diagnostic [V501](https://pvs-studio.com/en/docs/warnings/v501/) in [C/C\+\+ projects](https://pvs-studio.com/en/blog/examples/v501/) and [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) in [C\# projects](https://pvs-studio.com/en/blog/examples/v3001/) stem from this variable\-naming issue\. No [V579](https://pvs-studio.com/en/docs/warnings/v579/) warnings were issued for the other two projects\. **Assigning demerit points\.** MySQL is given 2 points\. Another similar bug was also found in MySQL\. ```cpp typedef char Error_message_buf[1024]; const char* get_last_error_message(Error_message_buf buf) { int error= GetLastError(); buf[0]= '\0'; FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, error, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR)buf, sizeof(buf), NULL ); return buf; } ``` **PVS\-Studio warning**: [V511](https://pvs-studio.com/en/docs/warnings/v511/) The sizeof\(\) operator returns size of the pointer, and not of the array, in 'sizeof \(buf\)' expression\. common\.cc 507 _Error\_message\_buf_ is an alias for an array of 1024 elements of type _char_\. There's one crucial thing to keep in mind: even if a function signature is written like this: ```cpp const char* get_last_error_message(char buf[1024]) ``` _buf_ is still a pointer, while the array size is only a hint to the programmer\. This means that the _sizeof\(buf\)_ expression works with the pointer here, not the array\. This results in passing an incorrect buffer size to the function \- 4 or 8 instead of 1024\. Again, no warnings of this type in Firebird and PostgreSQL\. **Assigning demerit points\.** MySQL is given 2 points\. ### Missing 'throw' keyword Here's another interesting bug \- this time in\.\.\. MySQL again\. It's a small fragment, so I'm giving it in full: ```cpp mysqlx::XProtocol* active() { if (!active_connection) std::runtime_error("no active session"); return active_connection.get(); } ``` **PVS\-Studio warning**: [V596](https://pvs-studio.com/en/docs/warnings/v596/) The object was created but it is not being used\. The 'throw' keyword could be missing: throw runtime\_error\(FOO\); mysqlxtest\.cc 509 The programmer creates an object of class _std::runtime\_error_ but doesn't use it in any way\. They obviously meant to throw an exception but forgot to write the _throw _keyword\. As a result, this case \(_active\_connection \=\= nullptr_\) can't be handled as expected\. Neither Firebird, nor PostgreSQL triggered any warnings of this type\. **Assigning demerit points\.** 2 demerit points are given to MySQL\. ### Calling the wrong memory\-deallocation operator The following example is taken from Firebird\. ```cpp class Message { .... void createBuffer(Firebird::IMessageMetadata* aMeta) { unsigned l = aMeta->getMessageLength(&statusWrapper); check(&statusWrapper); buffer = new unsigned char[l]; } .... ~Message() { delete buffer; .... } ..... unsigned char* buffer; .... }; ``` **PVS\-Studio warning**: [V611](https://pvs-studio.com/en/docs/warnings/v611/) The memory was allocated using 'new T\[\]' operator but was released using the 'delete' operator\. Consider inspecting this code\. It's probably better to use 'delete \[\] buffer;'\. Check lines: 101, 237\. message\.h 101 Block of memory for the buffer \(pointed to by the _buffer_ pointer, a member of class _Message_\) is allocated in a special method called _createBuffer_ by using the _new\[\] _operator, in accordance with the standard\. However, the class destructor deallocates the block of memory by using the _delete_ operator instead of _delete\[\]_\. No errors of this type were found in MySQL and PostgreSQL\. **Assigning demerit points\.** 2 demerit points go to Firebird\. ## Summing it all up Summing up the demerit points, we get the following: * Firebird: 1 \+ 1 \+ 2 \= 4 points\. * MySQL: 3 \+ 1 \+ 2 \+ 2 \+ 2 \+ 2 \= 12 points\. * PostgreSQL: 3 points\. Remember: the fewer points, the better\. And if you ask me \(a person with a wicked taste\), I'd prefer\.\.\. MySQL\! It has the most interesting bugs and it's the leader, which makes it a perfect choice for analysis\! Firebird and PostgreSQL are trickier\. On the one hand, even a one\-point margin counts; on the other hand, it's quite a small difference, especially because that point was given for a [V768](https://pvs-studio.com/en/docs/warnings/v768/) warning of the medium\-certainty level\.\.\. But then again, the codebase of PostgreSQL is way larger, yet it issued four hundred warnings at its automatically generated code\.\.\. Anyway, to figure out which of the two projects, Firebird or PostgreSQL, is better, we'd have to do a more thorough comparison\. For now, I put them on one podium place so no one is offended\. Maybe one day we'll compare them again more carefully, but it will be quite a different story\.\.\. So, the code\-quality rankings are as follows: * 1 place \- Firebird and PostgreSQL\. * 2 place \- MySQL\. ![0542_Firebird_MySQL_PostgreSQL/image7.png](https://import.viva64.com/docx/blog/0542_Firebird_MySQL_PostgreSQL/image7.png) Please remember that any review or comparison, including this one, is subjective\. Different approaches may produce different results \(though it is mostly true for Firebird and PostgreSQL, but not for MySQL\)\. So what about static analysis? I hope you are convinced now that it is useful for detecting defects of various types\. Want to find out if your codebase has any of those bugs? Then it's the right time to [try PVS\-Studio](https://pvs-studio.com/en/pvs-studio/download/)\! You write perfectly clean code? Then why not check your colleagues' code? ;\)