1. pointers/arrays/structures

I'm trying to go through a text file I'm passing and keep track of the numbers I'm reading in and the frequency they occurs in the 12 number file. But here's the thing, I only want to use pointers. The problem I'm running into is at the frequency part. When I compile and run the code, this is the output:
N: 10 F: 3
N: 10 F: 3
N: 8 F: 1
N: 7 F: 65
N: 5 F: 0
N: 4 F: 63
N: 10 F: 3
Segmentation fault (core dumped)

I had it printing out all the numbers before without it core dumping, but they were still wrong, as it is now (except the first 2). The N (number) is being stored in the hist struct array properly, but the F (frequency) is wrong after the first tens tens. I know the problem is in calcHistogram somewhere in the loops of chaos. Any tips would be greatly appreciated!

My guess would be that I'm going out of the range of the array somewhere, somehow. I simply can't find it though. Thanks in advance.

Code:

Code:
```#include <stdio.h>

struct freq {
int number;
int frequency;
};

//function prototypes
void displayScores(int* ar, int* count);
void calcHistogram(int* ar, int* count, struct freq* hist, int* ct);

int main() {
int ar[100];
int count = 0;
int ct = 0;

displayScores(ar, &count);

struct freq hist[count];

printf("\n");
calcHistogram(ar, &count, hist, &ct);
}

void readScores(int* ar, int* count) {

for (int i=0; i<11; i++) {
(*count)++;
scanf("%d", (ar+i));
}
}

void displayScores(int* ar, int* count) {
for(int i=0; i<*count; i++) {
printf("score %d: %d\n", i, *(ar+i));
}
}
void calcHistogram(int* ar, int* count, struct freq* hist, int* ct) {
int uniqueNumber;
for(int i=0; i<*count; i++){
uniqueNumber = *(ar+i);
for(int j=0; j<=*ct; j++){
if(uniqueNumber != (*(hist+i)).number){
(*(hist+i)).number = uniqueNumber;

}
if(*(ar+i) == *(ar+j)){
(*(hist+i)).frequency += 1;
*ct++;
}
}
printf("N: %d F: %d\n", (*(hist+i)).number (*(hist+i)).frequency);
}
}```

2. I don't have a computer to run your code at the moment, but Seg fault means that you have tried to access memory which you don't have permission to access

3. A few things -

"main" is not returning anything -> Very bad for your operating system.

You are not initialising all your variables

and most importantly, you have forgotten a "," in your arguments for that printf statement on line 54

4. Here's what I get when I try to compile the code you provided:
Code:
```\$ make foo
gcc -Wall -Wunreachable-code -ggdb3 -std=c99 -pedantic  -lm -lpthread  foo.c   -o foo
foo.c: In function ‘calcHistogram’:
foo.c:51: warning: value computed is not used
foo.c:54: error: called object ‘(hist + (unsigned int)((unsigned int)i * 8u))->number’ is not a function```
The second error appears to be due to a missing comma (,) between the second and third parameters to printf.

The first error is related to operator precedence. The postincrement (++) is higher than the dereference (*), thus you take the pointer ct, increment it (effectively changing the address it points to), then dereference it. But you don't do anything with the dereferenced value, hence the warning. The more disturbing thing is that ct (in calcHistogram) no longer points to the correct piece of memory. Thus, in your inner for loop, it keeps going until j reaches *ct, which is some unknown number since ct now points at garbage memory. That seems to be the cause of your seg fault. To fix it, wrap *ct in parentheses before incrementing (like count in your readScores function). Change line 51 to
Code:
`(*ct)++;`
Note, your algorithm still seems to have some bugs, but I'll let you work on that, wouldn't want to spoil the fun .

• You need to check the return value of scanf. If it only happens to store 4 elements successfully, your function will pretend it stored 11.
• It makes more sense to return the number of elements stored than to store it through a pointer.
• If the maximum limit changes from 11 to something else, you have to change your function's definition.

Code:
`int readscores(int *ar, int n) { int i; for (i = 0; i < n && scanf("%d", ar++) == 1; i++); return i; }`
Then call it like so:

It's unnecessary to pass pointers in most of the places that you have. The prototypes for your other two functions can be changed to the following:

Code:
```void displayscores(const int *ar, int n);
int calchistory(const int *ar, int n, struct freq *hist);```
*(ar+i)
...
(*(hist+i)).frequency
a[i], hist[i].frequency — they do the exact same thing and they're much easier to read.

Fixing those thing should make your code more compact and less confusing.

6. @Anduril462 ... thank you very much for that catch on the (*ct)++....i've been weary of that mistake in the past but i guess let that one slip by. fixing that now puts me back to where i was before with the following output:

N: 10 F: 2
N: 10 F: 2
N: 8 F: 1
N: 7 F: 64
N: 5 F: 1
N: 4 F: 64
N: 10 F: 3
N: 2 F: 2
N: 2 F: 32769
N: 8 F: 32769
N: 7 F: 32769

It's weird because some of the elements of the array are correct, for instance the 10, but others give me garbage...Really not sure what is going on

7. Code:
`(*(hist+i)).frequency += 1;`
What is the value in (*(hist+i)).frequency the first time, before you add 1? Hint, it's not necessarily zero.

I also would like say, I agree with Barney, that a[i] is much easier to read, and works the same as *(a+i). I didn't mention it because you wanted to use only pointers. However, there's absolutely no reason to use *(a+i), except as an academic exercise in how to write awkward, unreadable code.

8. Originally Posted by anduril462
Code:
`(*(hist+i)).frequency += 1;`
What is the value in (*(hist+i)).frequency the first time, before you add 1? Hint, it's not necessarily zero.

I also would like say, I agree with Barney, that a[i] is much easier to read, and works the same as *(a+i). I didn't mention it because you wanted to use only pointers. However, there's absolutely no reason to use *(a+i), except as an academic exercise in how to write awkward, unreadable code.
I think it's juts random garbage until it's initialized, which it isn't off the get go.... I'm trying to complete this program with only using pointers, not bracket notation so that's the reason behind my overuse of pointers

9. Correct, it's not initialized, so initialize it. Line 47 seems like a good place.

10. Originally Posted by anduril462
Correct, it's not initialized, so initialize it. Line 47 seems like a good place.
Awesome, that fixed the strange outputs...but now how could I go about displaying these without duplications, so only one "10" number would show up with frequency of "3" instead of three "10"s

11. Preferably in another function

12. I think the fact that you are displaying duplicates means you are not calculating your histogram correctly. Another function (or two) would be good to separate two different parts of your code. This could also help you isolate your problem and test your code.

First, create a function for printing the histogram data. Something like:
Code:
`void printHistogram(struct freq *hist, int histSize)  // note, histSize is not a pointer`
Make sure that works, so you can use it to see the contents of the histogram as you test your code. You can make a static array of histogram data to test:
Code:
```struct freq testHist[3] = {
{10, 2},
{1, 1},
{7, 3}
};
printHistogram(testHist, 3);```
Second, I would suggest a function like the following:
Code:
`int addNumToHistogram(struct freq *hist, int *histSize, int maxHistSize, int num)`
That function will search hist for num. If num is alread in the histogram, it will increment the count, otherwise it adds it to the end (assuming it doesn't exceed maxHistSize), with a count of 1, and increments *histSize. Use that function by itself (without calcHistogram), and simply insert 5 or so numbers with this function, with, perhaps one or two duplicates to test. Use the print function to make sure it looks correct. Like so:
Code:
```struct freq testHist[10] = {{0}};
int histSize = 0;
printHistogram(hist, histSize);```
Once you have those two functions working, the rest should be easy. You can simply call that addNumToHistogram from calcHistogram:
Code:
```void calcHistogram(int* ar, int* count, struct freq* hist, int* ct)
{
for i from 0; < *count
}```

13. Originally Posted by anduril462
I think the fact that you are displaying duplicates means you are not calculating your histogram correctly. Another function (or two) would be good to separate two different parts of your code. This could also help you isolate your problem and test your code.

First, create a function for printing the histogram data. Something like:
Code:
`void printHistogram(struct freq *hist, int histSize)  // note, histSize is not a pointer`
Make sure that works, so you can use it to see the contents of the histogram as you test your code. You can make a static array of histogram data to test:
Code:
```struct freq testHist[3] = {
{10, 2},
{1, 1},
{7, 3}
};
printHistogram(testHist, 3);```
Second, I would suggest a function like the following:
Code:
`int addNumToHistogram(struct freq *hist, int *histSize, int maxHistSize, int num)`
That function will search hist for num. If num is alread in the histogram, it will increment the count, otherwise it adds it to the end (assuming it doesn't exceed maxHistSize), with a count of 1, and increments *histSize. Use that function by itself (without calcHistogram), and simply insert 5 or so numbers with this function, with, perhaps one or two duplicates to test. Use the print function to make sure it looks correct. Like so:
Code:
```struct freq testHist[10] = {{0}};
int histSize = 0;
printHistogram(hist, histSize);```
Once you have those two functions working, the rest should be easy. You can simply call that addNumToHistogram from calcHistogram:
Code:
```void calcHistogram(int* ar, int* count, struct freq* hist, int* ct)
{
for i from 0; < *count
}```

I think it'd be cool if I could get it to add to frequency rather than fill in another N, like i just said, but in the same function..i feel like that is doable...Here's my function

Code:
```void calcHistogram(int* ar, int* count, struct freq* hist, int* ct) {
int uniqueNumber;
for(int i=0; i<*count; i++){
(*(hist+i)).frequency = 0;
uniqueNumber = *(ar+i);
for(int j=0; j<=*ct; j++){
if(uniqueNumber != (*(hist+i)).number){
(*(hist+i)).number = uniqueNumber;
}
if(*(ar+i) == *(ar+j)){
(*(hist+i)).frequency += 1;
(*ct)++;
}
}
printf("N: %d F: %d\n", (*(hist+i)).number, (*(hist+i)).frequency);
}
}```
And here is the output of this:

N: 10 F: 2
N: 10 F: 2
N: 8 F: 1
N: 7 F: 1
N: 5 F: 1
N: 4 F: 1
N: 10 F: 3
N: 2 F: 2
N: 2 F: 2
N: 8 F: 2
N: 7 F: 2

14. I want it to be sorted by F in descending order but with no duplicate N's

15. I think you missed the whole point of my post. I intended for it to merely increment the frequency if the number already existed, or to add it if did not exist yet. That would avoid duplicates. I was leaving the design/implementation for you to do. I didn't realize about the sorting, but that is a whole separate problem from removing duplicates. Don't try to do everything at once.