11
#define EXIT_SUCCESS 0
You are not permitted to define standard library names yourself. So just include <stdlib.h>, or omit the return from main() (remember, C will provide a success return if you don't provide one).
void fizzbuzz(int n) {
We could give that internal linkage:
static void fizzbuzz(int n) {
char result[9] = ""; // strlen(&...
9
Counting the digits doubles the amount of work, and the division/modulo is indeed very hard work for the CPU.
You know the maximum size of the resulting string, because it would be the largest or most negative integer you can be passed. So use a local array as a buffer, and then copy the result to allocated memory.
You have a separate function to reverse ...
9
it seems to be optimized quite well by the compiler, as it uses a lot of vectorized instructions
Well, kind of. GCC didn't use vectorization at all, and Clang used some of it but in a very strange way. Let's look at what it's doing. Basically I'm reviewing what Clang did, not so much your code.. but that then informs you about your code by proxy. I'll go ...
7
There are a few things that can be lifted out of loops.
The three screen related variables, screen_height, screen_width, and screen_size, along with scanline_in and scanline_out, can be initialized before the outer i for loop.
In the main blit loop, the check for scanline_out_index <= screen_size can be moved to before the x loop (and when false, you can ...
7
Besides lots of optimizations other people have suggested, you have two memory leaks.
The first is that, just before the final return in itoa(), you need to write free(digits);
The second is in the error handling. (A common place for errors BTW.) In particular, if getdigits returns successfully, but the malloc fails, you leak digits. The fix: Test digits ...
5
Memory management and optimization is important.
I will avoid any stylistic remark and focus on this particular aspect of your query.
Memory
First of all, the first rule of optimization is to avoid allocating memory. In general, memory allocation is slow-ish, and memory de-allocation even slower. In particular, most malloc implementation will perform "...
5
You have used static for the helper functions, to give them internal linkage. That's good.
return (1);
The parentheses don't add any value here. Just write return 1; like everyone else.
We can avoid the need to special-case n == 0 by always performing at least one division; use do/while in place of while:
int i = 0;
do {
n /= 10;
++i;
} ...
5
The interface
There are a number of useful things to return. Let's list them in order of which is least surprising:
The pointer to the start of the destination. This follows the standard library, but is least useful.
The pointer to the end of the destination. This follows an alternative convention embodies in the stpcpy() function and similar added to ...
4
You have a thorough review already; I'll just add some minor points.
We have a pointer to char in the cell structure:
typedef struct lv_cell {
// The text in the cell
char *text;
…
} lv_cell;
However, we don't need to modify the contents of the string, and making it const char* (and updating function signatures to match) would allow us to pass ...
4
Leak
digits is never free'd.
If either one of digits or nbr return NULL, the other allocation is lost.
digits = getdigits(n, digits_size);
...
nbr = malloc(digits_size + 1 * sizeof(char));
if (nbr == NULL || digits == NULL)
return (NULL);
Concept error
sizeof(char) is 1 so not much of a issue, yet sizeof(char) should multiply by the sum.
// malloc(...
4
You can use Godbolt to play around with the C code and see if anything makes the assembly look particularly better. (Seems like Clang is strangely much better than GCC at making this code branch-free.)
Technically, (a ^ b) has implementation-defined behavior if you end up XORing non-zero sign bit(s) together. It's better (in terms of portability) to write (a ...
3
The standard functions in string.h are not using malloc and free almost all. Therefore, the interface of itoa might be changed like this:
/* NG - because itoa returns a new memory */
char *itoa(int n) { ... }
/* OK - because itoa uses an existing memory */
void itoa(char *s, int n) { ... }
3
Brute-force searching is severely inefficient.
Try for a better algorithm by observing the problem carefully:
Algorithm:
Finding a way to easily find the n-th smallest palindrome number begins with a basic question:
How many are there for a given number of digits?
digits
count of palindrome numbers
0
none (or does zero count?)
1
9 (or 10 with zero), 1-...
3
Your chmod needs to go away; d.sh should already have the correct permissions, including when extracted from a source archive
You should probably delete all of your output diagnostic fluff.
f.c and d.sh are very bad filenames. The former should probably be renamed to fractal.c or somesuch.
You've mis-used make. The whole point of make is to express a ...
3
if I can make some improvements to it or if there are any error.
Lack of error checking
fopen(), malloc(), ftell(), fread(), (all I/O functions), etc. deserve to have their return values checked for errors.
In particular when a file may fail to open or memory fail to allocate, handle such cases to not forget the other. e. g.: do not forget to fclose(), or ...
3
Because you test for SPM_UNIX_ first.
That test includes test for the value of:
|| defined(__MINGW32__) \
|| defined(__MINGW64__) \
|| defined(__CYGWIN__) \
|| defined(__CYGWIN32__)
If any of these are true then you have a SPM_UNIX
Which means that these tests:
&& !defined(__MINGW32__) && !defined(__MINGW64__) \
&&...
3
How I Review Code
I have performed Code Reviews for over 30 years both on the Code Review Community and professionally. I have also had to maintain code written by others and this generally requires a thorough code inspection before starting.
The criteria I use for code reviews is:
Does the code compile without errors and warnings?
Does the code work as ...
3
Weird enum values
Why does stmfrm_state start at 0x8000, and then goes down? Also, if the enum names all start with SMFRM_WAIT_AA and only have a different number at the end, this tells me you shouldn't be using an enum here to begin with, and instead you should use an unsigned int counter.
Indeed it looks like it's just counting how many bytes of the header ...
3
Broken corner cases
"The first function will always try to correct the final result, " is unsupported, due to undefined behavior, when mod + b overflows as in floor_mod1(1, INT_MAX).
I think that in general the second function will be faster,
Speed should be a secondary concern, get the function to work correctly for all int a, int b.
Make code ...
3
It looks pretty good to me. I still have some notes to make.
Let's start with, there are 5 arithmetic operations (per row) to do the work that 4 FMAs could do. How good or bad is that? Well basically not bad at all in the context of just a simple 4x4 multiplication, because there are also 8 loads for those 5 arithmetic operations, so the problem is the loads....
2
main()
void
main()
Oops: that's not a standard signature for main(). You probably meant int main(void).
We have a memory access bug here, that's illuminated when we run under Valgrind:
char arr[10] = "abcdeabcde";
int n = strlen(arr) - 1;
arr doesn't have a terminating null. It's better to use an array of unspecified size, so we can be ...
2
char* log_filepath = (char*) malloc(sizeof(char) * path_len);
malloc() returns a void*, which in C converts to any object-pointer type (unlike in C++, if you're used to that). So the cast is unnecessary (it's slightly harmful, in that it distracts attention from more dangerous casts). Also, because char is the unit of size, sizeof (char) can only be 1, so ...
2
You have a great review by pacmaninbw, but I'll just pick up on one aspect and take it a little further.
There's some quite involved machinery for printing error messages:
struct data_structure_errordesc{
const int32_t code;
const char *message;
};
const struct data_structure_errordesc errordesc[] = {
{E_SUCCESS, "No error" },
{...
2
int is_palendrome(char *arr,int n,int l,int r)
Spelling: palindrome
Consider including <stdbool.h> and returning a bool value.
The arguments could do with more descriptive names. It's particularly unclear what n is for, as it appears to be unused within the function.
Why does arr point to modifiable char? I think we should pass const char*.
l and r ...
2
Description
"Finds largest palindromic sub-strings in a string without repeating any character"? Sure? Maybe "splits the string into the palindromic substrings"? Because, say, "ababba" would be spited into "aba", "bb", "a" - not "a", "b", "abba" to have the largest ...
2
First impression is pretty good - care taken with exception conditions and thought to the interface.
Headers: I find it helpful to have a consistent order for headers, to quickly see if the header I need is already included. For this, I group the Standard Library headers in alphabetical order. You might find that helpful too.
char *ptr = strstr(src, orig);...
2
should I worry about dealing with overflow in the computation of ord?
Yes. The difference may overflow.
Instead compare:
// Compute cross product sign (see next bullet)
int64_t ba = b[0]*a[1];
int64_t ab = a[0]*b[1];
if(ba < ab) return -1;
if(ba == ab) return 0;
return 1;
can I simplify this comparison function even further?
Excessive comparing is done.
...
1
Early returns
You have a bunch of them. I'm all for them (not everyone is), but you're inconsistent in your else style after an early return. Sometimes you do this:
if(a[1] == 0){
return a[0] > 0 ? 1 : 5;
}else // ...
and sometimes you dispense with the else:
if(a[1] > 0){
return a[0] > 0 ? 2 : 4;
}
return a[0] &...
1
The obvious issue with this code is the lack of structure. There are two main computations, given that we've already spotted that we can add all the exponents and perform a single exponentiation with the sum:
Finding the Lth and Rth odd-length palindromic numbers
Modular exponentiation
Each of these should be separate - independently testable - functions; ...
1
Forget about code (until the very end) and just consider how you could generate a sorted list of palindromes (of odd length) "by hand".
The first (1-digit) palindromes are 0, 1, ..., 9 i.e. the palindrome at position n (0 ≤ n < 10) in the list is n.
After that come 101, 111, 121, ..., but finding them by hand gets tedious (especially since the ...
Only top voted, non community-wiki answers of a minimum length are eligible
