This is C. You can't just call printf() and expect it to do the work for you. You need to iterate through the array with a for() or while() loop and print out each element.
Assuming it is an array of strings:
int i;
for (i=0; i<size; i++)
printf("array[%d] = %s\n", i, array[i]);
Answer from Jonathon Reinhart on Stack OverflowThis is C. You can't just call printf() and expect it to do the work for you. You need to iterate through the array with a for() or while() loop and print out each element.
Assuming it is an array of strings:
int i;
for (i=0; i<size; i++)
printf("array[%d] = %s\n", i, array[i]);
Please check this code. I believe that it does what you are expecting.
#include "stdio.h"
#include "stdlib.h"
#include "string.h"
int main(int argc, char** argv)
{
int size = 0;
int i;
char **array = malloc(0); //malloc for dynamic memory since the input size is unknown
static const char filename[] = "input.txt";
FILE *file = fopen(filename, "r");
if (file != NULL)
{
char line [ 128 ];
char delims[] = " ";
char *result = NULL;
while (fgets(line, sizeof line, file) != NULL)
{
result = strtok(line, delims); //separate by space
while( result != NULL )
{
size++;
array = realloc(array, size * sizeof (char*)); //declare array with unknown size
array[size - 1] = malloc(100 * sizeof(char)); // allocate memory for 100 chars
strcpy(array[size - 1], result); // copy the result
result = strtok( NULL, delims );
}
}
fclose(file);
}
else
{
perror(filename);
}
// return 0;
for (i = 0; i < size; i++)
{
printf("array[%d] = %s\n", i, array[i]);
}
// printf(array); //print array???
return (EXIT_SUCCESS);
}
Initializing and printing an array using pointers and dynamic allocation in C - Code Review Stack Exchange
c - dynamic array allocation and printing the elements - Stack Overflow
C - how are dynamic arrays printed? - Stack Overflow
c - Dynamic Array printing - Stack Overflow
Videos
There are a number of things I would change here, so I'm going to go through the process incrementally, applying changes to the whole file in passes rather than in chunks. I think this should make it more clear why the changes are implemented.
C99 Note
You should have access to, at the very least, a C99 compiler. If that's not true, you can ignore this first point, but if it is, you should change your counter declarations to be inside the first for clause.
for (int i=0;i<n;i++)
Pointers
In initArr, you both return the int array and assign it to the provided out parameter, arr. Why? Out parameters make code confusing and hard to follow! They make sense if you're performing the array allocation somewhere else, but since you're performing the array allocation inside initArr, just drop the arr parameter and return the value.
As a side effect of this, since you no longer need a double pointer, you can replace your dereferences with simple array accesses.
int * initArr (int n)
{
int * arr = (int *) malloc(sizeof(int*)*n);
printf("size is %d\n", sizeof(arr));
for (int i=0;i<n;i++)
{
arr[i]=i+10;
}
return arr;
}
Next, why are you passing a double pointer (int**) to printArr? More importantly, why does printArr have a return value if it's a purely side-effectful function? Change the parameter to int* and make the function return void.
void printArr (int * arr, int n)
{
for (int i=0;i<n;i++)
{
printf("Arr element %d is %d\n", i, arr[i]);
}
}
This also simplifies the main function.
int main(void) {
int * arr2 = initArr(10);
printArr(arr2,10);
return 0;
}
Next, let's take a look at the allocation itself (the one in initArr). First of all, you cast to (int *) manually, which is unnecessary and downright discouraged in C. If you're using C++, it's necessary (though you shouldn't need malloc in C++, anyway), but with a C compiler, just drop it.
Second of all, you are not actually allocating the right data! You're allocating n slots for int* valuesโint pointers. You actually want int data. This might not matter depending on your architecture and compiler, but it's still poor code. Fortunately, you can actually fool-proof thisโdon't pass an explicit type of sizeof at all! Just deference arr itself, and the compiler will calculate that value's size.
Those changes simplify the allocation to this:
int * arr = malloc(sizeof(*arr)*n);
Finally, the line just after thatโthe printf lineโis useless. It will always print the same value because it's checking the size of a pointer, which is always the same size regardless of type (though it can change on different architectures). That said, the line doesn't make any sense there. A function called initArr shouldn't have side effects, anyway. Just take it out.
Style and Warnings
I'm not sure if you just omitted it from your code, but your code depends on functions declared in external header files in the standard library. Include these at the top of your file.
#include <stdlib.h>
#include <stdio.h>
Next, let's talk about code style. You are writing some very densely-packed expressions in some places. Some whitespace can make code much more readable! For example, change the for loops to this:
for (int i = 0; i < n; i++)
Similarly, change your allocation to this:
int * arr = malloc(sizeof(*arr) * n);
Proper spacing makes code more readable and therefore more maintainable!
Finally, let's talk about pointer declarations. You are declaring your pointers with the asterisks just sort of "floating". This is actually not a bad compromise. Some people prefer them to be grouped with the type (int* foo), others with the name (int *foo). I prefer the former style, so in my final copy of the code, I changed them accordingly, but this is often merely personal preference.
Result
Here is all the code as it stands with the above changes implemented! Not only is it more readable due to style, but it's easier to understand the control flow since it no longer unnecessarily indirects variables via reference.
#include <stdlib.h>
#include <stdio.h>
int* initArr(int n)
{
int* arr = malloc(sizeof(*arr) * n);
for (int i = 0; i < n; i++) {
arr[i] = i + 10;
}
return arr;
}
void printArr(int* arr, int n)
{
for (int i = 0; i < n; i++)
{
printf("Arr element %d is %d\n", i, arr[i]);
}
}
int main(void) {
int* arr2 = initArr(10);
printArr(arr2, 10);
return 0;
}
Extra Notes
Can I use
arr[i]anywhere in the functions instead of**arr? (Gave error when I tried it)
You can! But * has lower precedence than array subscript ([]), so you'd need to use grouping operators
(*arr)[i]
This became unnecessary with the other changes, though.
What is the most common way of dealing with arrays when writing professional code? Is my approach correct?
Depends on the situation. Honestly, the best away to handle arrays is to not use themโif you're using a language with any more power than C, you should have higher-level language constructs (std::vector, ArrayList, etc.). However, if you have to use C, abstracting them behind an API is a good start.
One issue I'm encountering is having no way to tell printArr() what the size of the array is.
Yes, in C, arrays are just blocks of memory, so you can't get their length at runtime because it's not stored anywhere! If you wanted to do this, though, you could encapsulate your array in a struct.
typedef struct my_array {
unsigned int length;
int* data;
} my_array_t;
You could then return a my_array_t from initArr and pass a my_array_t to printArr, which could use arr.length to get the length.
Return values
initArrcommunicates the start address via both the return value and a pass-by-reference parameter. It surely is redundant. Normally you'd pick one of two possible signatures:int * initArr(int size); void initArr(int ** arr, int size);
and stick to it.
printArr, as the name suggests, only produces a side effect of values being printed. It has nothing to report back except possibly errors encountered while printing. Make itvoid printArr(int *, int).
(Ab)use of
sizeofinitArrallocates an array ofnunits of a pointer size. This works by coincidence: in pretty much any platform space taken by a pointer is enough to accomodate an integer. However it is not guaranteed.printf("size is %d\n", sizeof(*arr))is totally misleading. It is equivalent toprintf("size is %d\n", sizeof(int *)), and doesn't depend on how many bytes are actually allocated. Try to pass size 0 toinitArr.
a=(int*)realloc(t,sizeof(int)); You are trying to reallocate dynamic memory to just one integer size every time in the loop. Instead take a local variable count for number of elements to read in as,
int count = 0;
do
{
printf("enter the element");
scanf("%d",&n);
printf("%d",a[i]);
a = (int*)realloc(t, count * sizeof(int));
a[count - 1] = n;
i++;
} while(i<4);
free(a);
Lots of issues:
t=(int*)malloc(sizeof(int));
Don't cast the result of malloc (in C); it's unnecessary, and in older compilers it can suppress a useful diagnostic. Similarly, use the size of the actual object, rather than the type; that will save you some maintenance headaches:
t = malloc( sizeof *t );
Cleaner, easier to read, and not prone to issues if you decide to change the type of t. Although, given your code below, I think you meant for t to be a.
do
{
printf("enter the element");
Standard output is line buffered, meaning output is held until either the buffer is full or you send a newline character. To make sure your output shows up immediately, use fflush:
fflush( stdout );
scanf("%d",&n);
*a=n;
Has a been properly initialized to point anywhere yet? t and a are different objects, so assigning to t doesn't mean a is pointing anywhere meaningfule. Secondly, *a is equivalent to a[0]; are you sure you don't want to write
a[i] = n;
especially given the line:
printf("%d",a[i]);
a=(int*)realloc(t,sizeof(int));
Same as above; drop the cast, use the size of the object itself:
a = realloc( t, sizeof *a );
BUT, this has problems too. You're not actually extending your array; you keep allocating the exact same amount of space as when you started. And you keep using the same starting pointer, which may be different from your most recent result.
i++;
} while(i<4);
Here's what I think you're tryihng to do:
a = malloc( sizeof *a );
i = 0;
do
{
printf( "enter a the element: " );
fflush( stdout );
scanf( "%d", &a[i] );
printf( "%d\n", &a[i] );
int *tmp = realloc( a, sizeof *a * (i+2) ); // +2 since i starts from 0
if ( tmp ) // the array by one element
a = tmp; // only assign if realloc succeeded
} while ( ++i < 4 );
As a rule, extending an array one element at a time is expensive and inefficient; normally, you'd extend the array by some number of elements at once, reducing the number of times you have to call realloc:
size_t count = START_SIZE;
T *a = malloc( sizeof *a * count );
...
if ( idx == count )
{
T *tmp = realloc( a, sizeof *a * (2 * count) ); // double the size of the buffer
if ( tmp ) // each time we run up against
{ // the limit.
a = tmp;
count *= 2;
}
}
a[idx] = new_value;
You are accessing "elements" out of the boundary of arr, this is undefined-behavior; that's why you get values that are seemingly random (but the application could just as well crash).
You're accessing past the end of the array, which is undefined behavior. Technically anything can happen when you do that, but there are two things that typically happen in practice: either the program will crash, or you'll just get the value of whatever bytes are located in memory past the end of the array. (But this isn't something that you can, or should, rely on.)
With dynamic arrays you need to maintain a pointer to the beginning address of the array and a value that holds the number of elements in that array. There may be other ways, but this is the easiest way I can think of.
sizeof(list) will also return 4 because the compiler is calculating the size of an integer pointer, not the size of your array, and this will always be four bytes (depending on your compiler).
YOU should know the size of the array, as you are who allocated it.
sizeof is an operator, which means it does its job at compile time. It will give you the size of an object, but not the length of an array.
So, sizeof(int*) is 32/62-bit depending on architecture.
Take a look at std::vector.
You increment num_places at the wrong time.
Change
num_places++;
weap_List[num_places] = new_weap;
to
weap_List[num_places++] = new_weap;
Some quick thoughts:
for (i = 1; i < num_places; i++) {
num_places, being an array index, will start at 0. This loop will fail badly with only zero or one weapons in your list. (The condition i < num_places is checked after the first iteration of the loop body. If weap_List has one element (at index 0), this loop will access memory that is not allocated and not print the weapon. Start array-handling loops at 0. The only times you should ever tolerate one-indexed arrays is when moving a huge amount of FORTRAN code to C; when starting from scratch, use zero-indexed arrays, as K&R intended. :)
void *_tmp = realloc(weap_List, (num_allocated * sizeof(weapon))); weap_List = (weapon*)_tmp;
If _tmp were declared (weapon *) tmp = realloc(...), then you wouldn't need the (weapon *) cast on the next line.
Tiny nitpick: doubling the size is pretty expensive way to go when adding the 196609th weapon. Consider just growing by eight each time. (I'm guessing from your names that you're probably not going to be adding one item per second for weeks on end.)
Tiny nitpick: Avoid _prefix variable names, they are reserved for the C runtime environment. Yes, just about every program uses them, and yes, I've used them, and yes this one is probably safe :) but they are technically reserved. A plain tmp would suffice.
Larger nitpick: you're not checking the return value from realloc(3). It can fail. Your application should handle failure as gracefully as possible.
strcpy(list1.name.first, "abc");
strcpy(list1.name.second, "xyz");
These both will invoke undefined behaviour as first and second are declared as char variables , and you copy string literals to them .
You need to declare both of them as character arrays .
And this -
for (i=0; i < ((int)sizeof(&l)) /(int)sizeof(&l); i++) {
printf("%s\n", list1);
}
You try to print struct variable list1 with %s specifier, maybe you tend to print the strings that you wanted to copy. So directly print list1.name.first and list1.name.second in printf with %s specifier.
And the condition -
i < ((int)sizeof(&l)) /(int)sizeof(&l)
The cast is not necessary , and it will yield 1 so, loop will run for 1 time . Change the condition .
In your code, the member of structure name is defined as char. But you are trying to copy a string into it. May be this was a typo. If not you should define them as character array or character pointer. Also in your print statement you are trying to print structure data as string. It should be like -
printf("%s %s\n", list1.name.first, list1.name.second);
Also you assigned value 1234 to list.number. You may have meant list1.number. The parameters in function call of insert is wrong as well. And lastly, you have put l->data in functions init and insert which should be l->info.