qsort() and pointer problem

This is a discussion on qsort() and pointer problem within the C Programming forums, part of the General Programming Boards category; i'm trying to sort a array using qsort() but i just get junk printed. It probably has something to do ...

  1. #1
    Registered User
    Join Date
    Jun 2009
    Posts
    7

    qsort() and pointer problem

    i'm trying to sort a array using qsort() but i just get junk printed. It probably has something to do with my pointers but i can't find what's wrong.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include "lab41.h"
    person *personer[10];
    int fillen;
    
    
    int readfil(FILE *filen) {
        int counter = 0;
        int c;
        char mystring[100];
        person *p1;
    
        while((c = fgetc(filen)) != EOF) {
    			        p1 = malloc(sizeof(person));
    			        fscanf(filen,"%s", &p1->fnamn);
                        fscanf(filen,"%s", &p1->enamn);
                		fscanf(filen,"%d", &p1->alder);
                		fscanf(filen,"%d", &p1->vikt);
                		personer[counter] = p1;
                			
                			/*--------test ---------*/
                			
                            printf("%s", personer[counter]->fnamn);
                            printf(" ");
                            printf("%s", personer[counter]->enamn);
                            printf("\n");
                            printf("%d", personer[counter]->alder);
                            printf("\n");
                            printf("%d", personer[counter]->vikt);
                            printf("\n");
                            
                            /*--------slut test-------*/
                           counter++;
                           }
            fillen = counter;
    		printf("\n---------------co--END---------------\n");
        return 0;
        }
        
    int critter_cmp_namn (person *p1, person *p2)
    {
      return strcmp (p1->fnamn, p2->fnamn);
    }
    
        
    
    
    int skriv_ut_personer(FILE *filen, int val) {
    int counter = 0;
    	while(counter < (fillen-1) ){
    		if(val == 1) {
                            
                            fprintf(filen, "%s ", personer[counter]->fnamn);
                            
                            fprintf(filen, "%s\n", personer[counter]->enamn);
                            
                            fprintf(filen, "%d\n", personer[counter]->alder);
                            
                            fprintf(filen, "%d\n", personer[counter]->vikt);
    
    		} else if (val == 0){
                              fprintf(stderr, "%s ", personer[counter]->fnamn);
    
                            fprintf(stderr, "%s\n", personer[counter]->enamn);
    
                            fprintf(stderr, "%d\n", personer[counter]->alder);
    
                            fprintf(stderr, "%d\n", personer[counter]->vikt);
    			
    		}
    		counter++;
    	}
      
    
    return 0;
    }
    
    int meny() {
    int val;
    char filnamn[40];
    FILE *filen;
    
    
    printf("\nMENY\n");
    printf("1. Sotera efter namn\n");
    printf("2. Sotera efter alder\n");
    printf("3. Sotera efter vikt\n");
    printf("4. Skriv ut till fil\n");
    printf("5. Skriv ut till terminal\n");
    printf("Ditt Val: ");
    scanf("%d", &val);
    
    
    if(val == 1)
    {
    qsort (*personer, fillen, sizeof (person), critter_cmp_namn);
    meny();
    }
    else if (val == 2)
    {
    }
    else if(val == 3) {
    } else if(val == 4) 
    {
        printf("Fil att skriva till: ");
    	scanf("%s", &filnamn);
    	if((filen = fopen(filnamn, "w")) == NULL) {
                 printf("går ej att skapa/öppna fil");
                 meny();
            }
    	skriv_ut_personer(filen, 1);
    } 
    else if(val == 5) {
    skriv_ut_personer(NULL, 0);
    } else {
    
    	printf("felaktigt val");
    	meny(filen);
    
    }
    return 0;
    }
    
    void main() {
    int ett;
    FILE *filen;
    int val;
    char filnamn[40];
    enum filtyp {db,txt,bin};
    typedef enum filtyp filtyp;
    printf("Skriv filnamn: ");
    scanf("%s", &filnamn);
    
    if((filen = fopen(filnamn, "r")) == NULL) {
                 printf("file don't exist");
    	     exit(1);
            }
    readfil(filen);
    
    meny();
    close(filen);
    scanf("%d", ett);
    }

  2. #2
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,265
    Code:
    qsort (*personer, fillen, sizeof (person), critter_cmp_namn);
    should be
    Code:
    qsort (personer, fillen, sizeof (person), critter_cmp_namn);

  3. #3
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,058
    Without knowing the typedef of person it's really hard to say, but you may want to go over these two assignments again:
    Code:
    fscanf(filen,"%s", &p1->fnamn);
    fscanf(filen,"%s", &p1->enamn);

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    You should also be aware that main MUST return int, NOT void.

  5. #5
    Registered User
    Join Date
    Apr 2009
    Location
    Russia
    Posts
    116
    Code:
    person *personer[10];
    Code:
        qsort (*personer, fillen, sizeof (person), critter_cmp_namn);
    Code:
    int critter_cmp_namn (person *p1, person *p2)
    {
        return strcmp (p1->fnamn, p2->fnamn);
    }
    change it to

    Code:
    person *personer[10];
    Code:
        qsort (personer, fillen, sizeof(person *), (int (*)(const void *, const void *)) &critter_cmp_namn);
    Code:
    int critter_cmp_namn (person **p1, person **p2)
    {
        return strcmp ((*p1)->fnamn, (*p2)->fnamn);
    }

  6. #6
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    993
    The code c.user gave is close, but is still not correct. It should be:
    Code:
    int critter_cmp_namn(const void *a, const void *b)
    {
      person *const*p1 = a, *const*p2 = b;
      return strcmp((*p1)->fnamn, (*p2)->fnamn);
    }
    qsort(personer, fillen, sizeof(person *), critter_cmp_namn);
    qsort() expects a particular type of function pointer, and simply casting the wrong type of pointer is not the right way to go.

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,308
    Personally, I would prefer:
    Code:
    int critter_cmp_namn(const void *a, const void *b)
    {
        const person *lhs = *(const person**)a;
        const person *rhs = *(const person**)b;
        return strcmp(lhs->fnamn, rhs->fnamn);
    }
    /* ... */
    qsort(personer, fillen, sizeof(personer[0]), critter_cmp_namn);
    My first idea is to remove one level of indirection to make it a little easier to interpret the strcmp() call (but then one can argue that the burden of interpreting the two levels of indirection has merely shifted to the casts). The second idea is to take the sizeof(personer[0]) instead of sizeof(person*), which makes for one less thing to modify should the element type of personer change.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    993
    My first idea is to remove one level of indirection to make it a little easier to interpret the strcmp() call (but then one can argue that the burden of interpreting the two levels of indirection has merely shifted to the casts). The second idea is to take the sizeof(personer[0]) instead of sizeof(person*), which makes for one less thing to modify should the element type of personer change.
    I still can't decide which is "least worst". They both have their ugly qualities, but if my compare function has any sort of complexity, I'd likely do the same thing you did. Better to have one ugly initialization than ugly use all over the function.

    As for applying sizeof to the object rather than the type, I concur 100%. I can't really think of a good reason to use sizeof with a type name if it's not necessary.

  9. #9
    Registered User
    Join Date
    Apr 2009
    Location
    Russia
    Posts
    116
    I will correct my function a bit
    Code:
    int critter_cmp_namn (person **p1, person **p2)
    {
        if (!p1 || !p2 || !*p1 || !*p2)
            return 0;
        return strcmp ((*p1)->fnamn, (*p2)->fnamn);
    }
    it would be better

  10. #10
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Quote Originally Posted by c.user View Post
    I will correct my function a bit
    Code:
    int critter_cmp_namn (person **p1, person **p2)
    {
        if (!p1 || !p2 || !*p1 || !*p2)
            return 0;
        return strcmp ((*p1)->fnamn, (*p2)->fnamn);
    }
    it would be better
    Actually it would be terrible, because it returns 0 for NULL pointers, AND for a match.


    Quzah.
    Hope is the first step on the road to disappointment.

  11. #11
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Quote Originally Posted by c.user View Post
    I will correct my function a bit
    Code:
    int critter_cmp_namn (person **p1, person **p2)
    {
        if (!p1 || !p2 || !*p1 || !*p2)
            return 0;
        return strcmp ((*p1)->fnamn, (*p2)->fnamn);
    }
    it would be better
    That looks to me like that if-statement is attempting to do the job of an assert, so you should probably just use an assert.

    I wouldn't think you intend the ordering of NULL vs non-NULL item to be non-transitive. Imagine 3 items (a, b, and c). b is NULL, but a and c are not, and a is less than c.
    Comparisons of a,b, and c could lead us to conclude that b < b. Oops!
    The result of course is that the sorting function will probably not produce the desired result. In fact some sorting algorithms that rely on transitivity could get confused and even crash under those circumstances.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  12. #12
    Registered User
    Join Date
    Apr 2009
    Location
    Russia
    Posts
    116
    I tryed to leave the NULL in it's place (so now I see that the sorting will not correct though, hence it should return 1 for NULL and it is necessary to make something like errno (but local in the program) for the informing the programmer about this situation)
    then the programmer can interpret it as an error or not

  13. #13
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,308
    Quote Originally Posted by c.user
    I tryed to leave the NULL in it's place (so now I see that the sorting will not correct though, hence it should return 1 for NULL and it is necessary to make something like errno (but local in the program) for the informing the programmer about this situation)
    As a pre-condition to that comparator, null pointers should not be allowed. It is needlessly inefficient to check if the arguments are null pointers each time the function is called. As iMalc noted, an assert should be used to check the pre-condition, upon which the check can be turned off for a release build if necessary, e.g.,
    Code:
    int critter_cmp_namn(const void *a, const void *b)
    {
        const person *lhs = *(const person**)a;
        const person *rhs = *(const person**)b;
        assert(a && b && "Null pointers not allowed as arguments to person comparator.");
        return strcmp(lhs->fnamn, rhs->fnamn);
    }
    It is probably better to place the assert before the dereference, but I am assuming C90 instead of C99.

    There is no point checking for empty strings separately since strcmp() will already handle that case.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  14. #14
    Registered User
    Join Date
    Apr 2009
    Location
    Russia
    Posts
    116
    Code:
    char nullflag;
    
    ...
    
    int critter_cmp_namn (person **p1, person **p2)
    {
        if (!p1 || !p2 || !*p1 || !*p2)
            return nullflag = 1;
        return strcmp ((*p1)->fnamn, (*p2)->fnamn);
    }
    then after sorting

    Code:
        if (nullflag) {
            nullflag = 0;
            /* assert(nullflag && "Null pointers are not allowed in the sorting"); */
            for (personp = personer; personp != NULL; personp++)
                Print(personp);
            printf("with nulls\n");
        } else {
            int i;
            
            for (i = 0; i < sizeof personer / sizeof *personer; i++)
                Print(&person[i])
        }
    i.e. he can decide what to do

  15. #15
    Registered User
    Join Date
    Apr 2009
    Location
    Russia
    Posts
    116
    Quote Originally Posted by laserlight
    As iMalc noted, an assert should be used to check the pre-condition, upon which the check can be turned off for a release build if necessary, e.g.
    I am recommend to control the program, and you are recommend to debug the program
    if it will be turned off what will happen if there will an incorrect data in that array ?

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. qsort() in Array of Pointer to String
    By vb.bajpai in forum C Programming
    Replies: 8
    Last Post: 06-16-2007, 05:18 PM
  2. qsort() w/ pointer swapping
    By soopah256 in forum C Programming
    Replies: 10
    Last Post: 12-05-2003, 07:54 AM
  3. qsort and unions
    By doylie in forum C Programming
    Replies: 1
    Last Post: 01-31-2003, 12:13 PM
  4. qsort using function ptr
    By nomes in forum C Programming
    Replies: 5
    Last Post: 10-01-2002, 05:41 PM
  5. problem with incompatible pointer type
    By SIKCAR in forum C Programming
    Replies: 7
    Last Post: 09-14-2002, 05:42 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21