4

I need to be able to enter array of ints and hold it in a set inside a struct, however for some reason it won't read the numbers into the array:

#include <stdlib.h>
#include <stdio.h>
#include <stdarg.h>

#define MAX 100

typedef struct set {
    int arr[MAX];
} set;

set SETA;

int read_set(set,...);
void print_set(set);

int main(){
    int x;
    x=read_set(SETA,2,3,4,-1);
    printf("%d numbers were read\n",x);

    print_set(SETA);
    return 0;
 }

void print_set(set s){
    int *iptr;
    iptr=s.arr;

    while(*iptr++){
        printf("%d ",*iptr);
    }
}


int read_set(set s,...){
    va_list ap;
    int i=0;
    int c=0;

    va_start(ap,s);

    while( *ap != -1){
        s.arr[i++]=va_arg(ap,int);
        printf("%d was entered\n",s.arr[i]);
        c++;
    }
    va_end(ap);
    return c;
}

the output I get is:

0 was entered  
0 was entered  
0 was entered  
3 numbers were read  

and needless to say that print_set prints nothing.

Cœur
  • 37,241
  • 25
  • 195
  • 267
Milo
  • 131
  • 10
  • 1
    Related: https://stackoverflow.com/q/30980759/694576 – alk May 19 '18 at 07:17
  • Also you might like to read this: [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) – alk May 19 '18 at 07:24
  • @alk I will! thanks – Milo May 19 '18 at 07:25
  • This `while(*iptr++){` will run into undefined behaviour as long the reader function does not read a `0`. – alk May 19 '18 at 07:52
  • For some reason this does work , should I change it anyway? – Milo May 19 '18 at 07:57
  • Well undefined behaviour is undefined, code having invoked it, might seem to work, or crash, or what ever, anything may happen. To actually see that there is something wrong you want to run the program using a memory checker like [Valgrind](http://valgrind.org). – alk May 19 '18 at 08:02
  • I have heard of it, but having trouble installing it on Windows 10, any good guides? YouTube guides are all aimed at UNIX and MAC, been thinking for a long time switching to Linux, but have too much Windows apps that I use. – Milo May 19 '18 at 08:05
  • Valgrind is not available for plain Windows programming. Setup a VM running on your Windows box and inside the VM install Linux. Works fine for me. – alk May 19 '18 at 08:06
  • 1
    I do have VM with Ubuntu, will check into it , thanks – Milo May 19 '18 at 08:07

1 Answers1

3

In

while( *ap != -1){
    s.arr[i++]=va_arg(ap,int);
    printf("%d was entered\n",s.arr[i]);
    c++;
}

you increment i when you record the value. When you try to print s.arr[i] you are one ahead of where you stored the value.

Increment after the print?

while( *ap != -1){
    s.arr[i]=va_arg(ap,int);
    printf("%d was entered\n",s.arr[i]);
    i++;
    c++;
}

You function int read_set(set s,...) takes a copy of a set s and puts stuff in it. By the time you get back to the calling function in main, the set that you copied in is unchanged. You need to send pointers to variables to change them:

int read_set(set *ps,...)

and the calling code would then need to send the address x = read_set(&SETA, 2, 3, 4, -1); so you can change what's in the set. An alternative is to return the filled structure.

Two other things to think about. First, you could declare your set inside main - it has no reason to be global. And you don't need to captilaise it.

int main() {
    set setA; //style/design point. Also don't shout.
    //... etc
}

Also, look at your print function. It uses while (*iptr++), so is checking of 0s or some kind of NULL to stop looping. I can't see any zeros so this needs a re-think. And, do you want to have a set that won't display anything beyond a 0?

doctorlove
  • 18,872
  • 2
  • 46
  • 62
  • thanks! any ideas why the print_set won't print anything? – Milo May 19 '18 at 07:05
  • @YaroslavMiloslavsky: "*any ideas why the print_set won't print anything*" in 1st place because it is not called, at least not be the code you show. – alk May 19 '18 at 07:14
  • @alk I added it, in my code it does show, next time I will be clearer:) – Milo May 19 '18 at 07:15
  • @doctorlove I need a set of numbers between 0 and 128, the stopping sign is -1, I will rethink my strategy regard the topic. thank you for sharing your knowledge, it helped me a lot! – Milo May 19 '18 at 07:23
  • Does the read function actually save the -1? And the print while loop isn't checking *iptr against -1. – doctorlove May 19 '18 at 07:24
  • 1
    @YaroslavMiloslavsky: If `-1` is not defined as an allowed value for `arr`'s elements, then the approach you took is perfectly fine. – alk May 19 '18 at 07:24
  • @doctorlove no the function reads only until the next pointer is -1 and terminates – Milo May 19 '18 at 07:32
  • 1
    So the print function - or anything else - won't have a -1 to show you've reached then end. You could pass the length/count around along with the `set` instead – doctorlove May 19 '18 at 07:36
  • You are right , but the pointer will reach a null at some moment after there are no more numbers in the array – Milo May 19 '18 at 08:00
  • @doctorlove I improved my code by using the following: in read_set i added this line 'ps->arr[i]=-1;' and in print_set i added this 'while(*iptr != -1)' thanks for pushing me not to leave my code uncomplete – Milo May 19 '18 at 08:16