24
Typo
lenght is spelled length.
Magic numbers
What does 95 signify? You'll want to put this in a named #define or a const.
Allocation failure
After calling malloc, always check that you've been given a non-null pointer. Allocation failure does happen in real life.
Indentation
You'll want to run this through an autoformatter, because your if block has ...
17
This answer uses pointer-casting for type-punning just to save space. In practice keep using your union (safe in ISO C99, and in C++ as a GNU and MSVC extension) or memcpy (safe in C and C++). This pointer-casting is only safe in MSVC, or GNU-compatible compilers with -fno-strict-aliasing
Initial approximation
Packed bit fields are not only unnecessary ...
12
Naming
It's unconventional to name a type with all-uppercase - we normally reserve those names for preprocessor macros, to warn readers that they need treating with care. Avoid such names for ordinary identifiers.
Avoid using identifiers that begin with an underscore - in many situations, those names are reserved for use by the implementation, which could ...
10
Whilst your code works, there are a number of simplifications that you might try.
As Reinderien says, get rid of "magic" numbers
Having done that, declare a single string containing all 95 characters with the special ones last. This does away with all the strcat code.
It's good practice to declare has_special_characters as type bool. You will have to ...
8
FYI, in IEEE754 sqrt is a "basic" operation that's required to be correctly-rounded (rounding error <= 0.5ulp), same as + - * /. Hardware FPUs (I think) always provide sqrt if they provide the other operations, especially division which is typically implemented similarly (and with similar performance).
NR iterations each involving a division are not ...
8
I don't see why you define this constant yourself:
#define MANTISSA_SIZE 52
Given we already assume that FLT_RADIX is 2, we can use the appropriate macro from <float.h> (DBL_MANT_DIG for double, etc.).
I think there's danger of integer overflow here:
/* Divide the exponent by 2 */
r.o.e -= EXPONENT_BIAS;
r.o.e = (r.o.e & 1) | (r.o.e >> 1);...
7
Hi Jim Diroff II and welcome to CodeReview,
Your code looks sane and I didn't spot any leaks.
In printlist() why are you using a double pointer? This is only required if you intend to modify the pointer value, which you don't. Better use a normal pointer here:
void printlist(NODE *head)
{
NODE *tracer = head;
while (tracer != NULL)
{
...
7
Okay, what's the deal with #define ezs(x) (x)? Is it some kind of trick to either hide C-style casts, or make them more greppable? If the latter, why such a short name? How about #define CAST_TO(x) (x)?
Other naming comments:
was_read sounds like a boolean. I think you mean bytes_read or even num_bytes_read.
Your function names are all tersified according ...
7
You could get away without all the complicated memory allocation if you simply require that the calling code passes you the memory for the password. It could look like this:
#include <stdio.h>
#include <stdlib.h>
void generate_password(char *password, size_t password_size) {
for (size_t i = 0; i < password_size - 1; i++) {
...
6
Makefile review
We're missing the instruction to tell Make to remove partial results from failing rules; all Makefiles should somewhere contain
.DELETE_ON_ERROR:
makeiso is never up to date. We don't want to re-run it unless the binary has changed, so I'd write:
makeiso: $(RESULTING_ISO)
$(RESULTING_ISO): $(BINDIR)/$(BINARY)
$(info )
$(info $(...
6
Use descriptive variable names
Instead of int a, b, c, give them more descriptive names:
int day, month year;
sscanf() will ignore trailing garbage
The function sscanf() will stop parsing after the last conversion. So the string "1.2.3...." will be cleared by your check for digits and period characters, and then sscanf() will read 3 integers, and returns ...
5
Memory leak
Consider this (I removed the pointless cast and identity multiplication):
master[i] = malloc(sizeof ele);
master[i] = ele;
Here, we allocate memory (if malloc() succeeds), but then immediately overwrite our one and only pointer to it, so we're unable to ever free it. The first assignment should just be removed, leaving only master[i] =...
5
To complement the Makefile review, here are a few more points that may help you improve your program.
Use VPATH
make already has a number of builtin rules that could be used for this project with no loss of generality. If you omit the colored printing (which I would advocate) and use VPATH, you could use replace most of your Makefile with a single rule:
...
5
It's been touched on (e.g. fixed in Peter's example) but not explicitly stated by anyone - but to me, the most obvious issue is in the duplication of code.
You have the following if statement:
if(has_special_characters)
{
//codeblock 1
}
else
{
//codeblock 2
}
where codeblock 1 and codeblock 2 are almost exactly identical. In fact it seems that the ...
5
We need only 5 maximum elements, for that we don't need to sort whole array. We can use alternative approach
Approach 1: Use 5 local variables and keep track of minimum element in it.
Approach 2: Use Min-heap and insert first 5 elements of array in min-heap, then iterate on array from index = 5, if heap-root is smaller than number in array then extract ...
4
Modular odd check
The compiler is likely to do this anyway, but can't
if(c % 2 == 0)
be
if (!(c & 1))
?
Factoring
Isn't
r.f -= (r.f*r.f-a)/(2*r.f);
equivalent to:
r.f = (r.f + a/r.f)/2?
Optimization
Depending on which compiler you're using and what flags you pass it, some of your attempted math risks modification by the optimizer, and in an ...
4
There isn't going to be a significant difference between C and C++ when using the same algorithm.
Using a while loop versus a for loop in this case will not have a significant difference either. The loop is only going to sum 5 values.
The problem is the algorithm, as another answer pointed out do as much of the sorting as possible as the numbers are input. ...
4
NO: C++ is not magically slower than C. Bad C++ code is slower than good C code and vice versa.
NO: For the compiler it is completely irrelevant whether you use a for or a while loop as they will all be normalized into a consistent representation anyway.
Now to the actual review. Your code will not improve from porting to C as you are already not using any ...
4
regarding:
int main(int argc, char *argv[]) {
Since the parameters are not used, the compiler will output two messages about unused parameters. Strongly suggest using the signature:
int main( void )
regarding:
master[i] = (char *)malloc(sizeof(ele) * sizeof(char));
1) the returned type is void* which can be assigned to any pointer. Casting just ...
4
the posted code does not cleanly compile!
Compile with warnings enabled, then fix those warnings.
for gcc, at a minimum use:
-Wall -Wextra -Wconversion -pedantic -std=gnu11
Note: other compilers use different options to produce the same results.
in function: setup_myfile() there are statements like:
return -1;
However, that only gets execution ...
3
Remarks for beginners:
Never hide a pointer behind a typedef! Never! It makes the code confusing and very hard to read and maintain.
You can't unfortunately use a generic Node pointer like in your code, because this isn't well-defined by the language. There are ways around it, but they are a bit controversial (see advanced reply below).
Generally, when ...
3
I see some things that may help you improve your program.
Omit unused variables
Because argc and argv are unused, you could use the alternative form of main:
int main()
Eliminate global variables where practical
Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for ...
3
Please improve the variable names - it's not at all obvious what a, k and p are (and conventionally, p is usually used for a small-scope pointer, not an integer).
Always check whether malloc() (or calloc(), or realloc()) returns a non-null pointer before dereferencing.
The algorithm is so opaque that I'm not going to attempt to unravel it. It's nowhere ...
2
A useful idea, and very well implemented.
I'd re-write this condition:
while (!(index < buflen))
as
while (buflen <= index)
Apart from that, I found the code very readable.
It's a good idea to store the received data as int rather than converting to char immediately (though I do worry somewhat about using a signed type in code_point - perhaps its ...
2
main()
It looks like this line it trying to allocate some memory to store one of the morse code strings (e.g., "-...").
master[i] = (char *)malloc(sizeof(ele) * sizeof(char));
However, ele is a char* and sizeof(char) is 1. So, this allocates memory for one char*, not enough to store a string of up to 4 char plus a terminating NULL. To get the length of ...
2
Is this common practice?
Enum-based type tracking in C? Yes.
Would there be a better way to do it?
Depends on a few things, including your definition of better. I think this is fine, but if your type-conditional code ends up being extremely long, then you can move to a more C++-style approach, where instead of tracking a type enum, you track function ...
2
Major stuff
Consider const
Design: kth_largest() has a side effect of re-arranging array. this is surprising and not part of "Finds the kth largest element of an array in O(n) time." I'd expect code to do the job without the side effect
// long kth_largest(size_t k, long *array, size_t n_elts) {
long kth_largest(size_t k, const long *array, size_t n_elts)...
2
static functions
Both of your getNameNumber and printNumber should be made static, because I doubt that you plan to export them.
Simpler functions
printf("Enter the amount of people: ");
can simply use puts, which doesn't have any formatting code in it. You use puts below, so this would also help with consistency.
Stack allocation
person people[size];
...
2
Convenience typedefs
Consider adding convenience typedef declarations on your structs and enums, i.e.
struct Value {
// ...
};
becomes
typedef struct ValueTag {
// ...
} Value;
You've already done this with Stack, though I recommend renaming the tag:
typedef struct TagStack
{
struct Value *array;
size_t used;
size_t size;
} Stack;
...
2
There is not much code so I won't write much review
The issue that makes me most uneasy is the interface decision, specifically requiring calling code to calculate len in bytes.
If I am in "trying my best mode" as a coder, late at night with a stressful customer deadline hanging over me, it'll be all I can manage to remember to clear memory. I will ...
Only top voted, non community-wiki answers of a minimum length are eligible
