What is wrong with this c-program?

Discussion in 'C Programming' started by Albert van der Horst, Dec 25, 2013.

  1. I have the following program that gives consistently a segfault
    with
    --------------------
    46344 1
    46345 1
    46346 1
    46347 1
    46348 1
    46349 0
    Segmentation fault
    -------------------

    The program itself doesn't give any complaints with gcc 4.4
    on a stable Debian.
    It is a classical prime sieve.
    I can't find any fault with it.

    --------------------------

    #include <stdio.h>
    #include <assert.h>
    #include <inttypes.h>

    #define BYTE uint8_t

    /* largest number */
    #define MAXSIZE 20000000
    #define SIZE 2000000

    long int TheLimit;

    /*****************************************************************************/
    /* */
    /* Fill the `nprimes' table up to `n' */
    /* */
    /*****************************************************************************/


    static BYTE composite[ MAXSIZE+1];

    void fill_primes( int n)
    {
    int i,j;

    assert(n<=MAXSIZE);
    for (i=0;i<=n; i++) composite = 0;
    for ( i=2; i<=n; i++)
    {
    printf("%d %d \n",i,composite);
    if (!composite)
    {
    for (j=i*i; j<=n; j+=i) composite[j] = 1;
    }
    }
    }

    int main( int argv, char** argc)
    {
    int i;
    TheLimit = atoi(argc[1]);
    printf("n= %d\n",TheLimit);
    fill_primes(TheLimit);
    printf("primes ready\n");
    }

    ------------------------------


    Must I suspect the installation of my compiler?

    Groetjes Albert
     
    Albert van der Horst, Dec 25, 2013
    #1
    1. Advertisements



  2. 1. you fail to handle the case of a missing argument.
    2. what is the size of int j? what is the maximum value an int can hold?
    3. what is the value of 46349*46349 ?
    4. compare the answer to 3 with that of 2b.
    5. your compiler is fine. your code needs repair.
     
    D. Aaron Sawyer, Dec 25, 2013
    #2
    1. Advertisements

  3. Albert van der Horst

    Luuk Guest



    46349 * 46349 = -2146737495
    you have an overflow,
     
    Luuk, Dec 25, 2013
    #3
  4. Get yourself a debugger:

    gdb)$ run 60000
    .......
    46348 1
    46349 0

    Program received signal SIGSEGV, Segmentation fault.
    0x080485f0 in fill_primes (n=60000) at nprim.c:34
    34 for (j=i*i; j<=n; j+=i) composite[j] = 1;
    (gdb)$ print j
    $1 = -2146737495
    (gdb)$


    You will probably find that 46349*46349 > 2^31.
     
    Øyvind Røtvold, Dec 25, 2013
    #4
  5. Øyvind Røtvold wrote:
    [...]
    Perfect answer, kudos ! :)
     
    Ralph Spitzner, Dec 25, 2013
    #5
  6. Albert van der Horst

    Geoff Guest

    You're not looking very hard. See below.
    -----------^^^^^^^^^^
    Non-standard header used.
    Why do this? Why not just use standard type unsigned char?
    This is never referenced again, delete it.


    Unused variable i.
    No, your compiler is fine, your usage of it is flawed.

    Use the -Wall switch and your compiler will talk to you.
     
    Geoff, Dec 26, 2013
    #6
  7. Albert van der Horst

    Noob Guest

    inttypes.h /is/ a standard C99 header.
    https://en.wikibooks.org/wiki/C_Programming/C_Reference/inttypes.h
    @Geoff: unsigned char /may/ be wider than 8 bits.

    @Albert: a typedef seems more appropriate than a macro.
    typedef uint8_t byte;
    Allocating INT_MAX+1 ints is unlikely to succeed (except on Win64)
    Some people also add -Wextra -O2 to enable even more warnings.
     
    Noob, Dec 26, 2013
    #7
  8. And if it is, then uint8_t will not exist. (Which means the program
    will fail to compile; if it depends on 8-bit bytes, that's probably a
    good thing.)

    [...]
     
    Keith Thompson, Dec 26, 2013
    #8
  9. Albert van der Horst

    BartC Guest

    Because 'byte' (as I would write it) is a lot shorter (and easier on the eye
    than 'uint8_t'). And it explains more, because everyone knows what a byte
    is. Also that a byte* type probably isn't going to used for strings.
     
    BartC, Dec 26, 2013
    #9
  10. Boys, boys, boys!

    You guys really need to get a room!

    --
    Here's a simple test for Fox viewers:

    1) Sit back, close your eyes, and think (Yes, I know that's hard for you).
    2) Think about and imagine all of your ridiculous fantasies about Barack Obama.
    3) Now, imagine that he is white. Cogitate on how absurd your fantasies
    seem now.

    See? That wasn't hard, was it?
     
    Kenny McCormack, Dec 26, 2013
    #10
  11. Albert van der Horst

    Noob Guest

    No. Use "octet" if you need a word for uint8_t, not "byte".

    "byte" is a pun on "bit/bite". It conveys no size information.
    I don't think anyone would use (unsigned char *) for strings either.
     
    Noob, Dec 26, 2013
    #11
  12. Albert van der Horst

    Seebs Guest

    It might be easier on the eye, but it's also unfamiliar.
    C programmers had better know what an unsigned char is. Furthermore,
    "byte" has a lot of implications which appear to be possibly-false and
    definitely-irrelevant to your program.
    But that's misleading, because of course it could be.

    -s
     
    Seebs, Dec 26, 2013
    #12
  13. Albert van der Horst

    BartC Guest

    No? If you had to make an educated guess as to the number of bits in a
    'byte' type, and your life depending on it, what would be your best guess;
    11?

    I always liked to think, myself, that a bit was an eight of a byte, in the
    same way that a bit is an eighth of a dollar.

    But apart from my opinion, the Wikipedia article suggests that an 8-bit byte
    is the 'de factor standard'; that it has ubiquitous acceptance (other than
    in c.l.c obviously); and that that meaning is ratified by a formal
    international standard.

    While 'octet' is used where unambiguity is needed (ignoring by the ambiguity
    caused by people who've never heard of an octet).

    Anyway, if the width of a 'byte' type alias in C source code is ambiguous,
    then so is the width of a 'char' type.
    I use a macro 'uchar', with strings being uchar*, since I don't want any
    nasty surprises from the possible signedness of a char type.
     
    BartC, Dec 26, 2013
    #13
  14. Why is "uchar" a macro rather than a typedef?

    And what's the advantage of "uchar" over "unsigned char"? If I see
    "unsigned char" in your code, I know exactly what it means; if I see
    "uchar", I'm only about 98% sure.
     
    Keith Thompson, Dec 26, 2013
    #14
  15. Albert van der Horst

    James Kuyper Guest

    On 12/25/2013 11:46 PM, Martin Ambuhl wrote:
    ....
    They know, from a user perspective, that defective software is
    commonplace, so they suspect the compiler. They have not yet learned
    that the reason why defective software is commonplace is that it's very
    hard to write defect-free code, so they underestimate the likelihood
    that it's a mistake in their own code.
     
    James Kuyper, Dec 26, 2013
    #15
  16. Albert van der Horst

    BartC Guest

    98% is good enough. It helps stop the code from sprawling and saves typing.
    I'm sure you also appreciate C choosing 'int' and 'char' rather than
    'integer' and 'character'.
     
    BartC, Dec 26, 2013
    #16
  17. Ok. Not the way I'd do it, though. And a bit of practice should
    alleviate the problem of remembering how typedefs are defined.
    Not particularly, no. I've used languages that spell out their type
    names, and I've never has any particular problem with it.

    On the other hand, in C "integer" and "character" are more generic terms
    (there are several integer types, of which "int" is just one; likewise
    for character types). That might be considered a rationale for using
    the shorter names, though I don't believe that was the original reason.

    98% tends not to be good enough *for me* when I can get 100% at the
    trivial cost of a little extra typing. YMMV.
     
    Keith Thompson, Dec 26, 2013
    #17
  18. Albert van der Horst

    Geoff Guest

    I stand corrected. I looked it up in N1570 and saw I was mistaken only
    after I had sent the post. I blame the eggnog. :)
    A good time to use the #error directive then, isn't it?
    More eggnog abuse. Upon review he's not using that value as I
    initially perceived him to be using it.
    Agreed.
     
    Geoff, Dec 26, 2013
    #18
  19. Albert van der Horst

    Geoff Guest

    Upon further review, I were rewriting his code I'd bypass the byte vs.
    BYTE vs. char issue altogether and use bool for the type of
    composite[] because that's what he's using it as. It has only two
    values and he could write it much more clearly using true and false.
     
    Geoff, Dec 26, 2013
    #19
  20. Albert van der Horst

    Geoff Guest



    A lovely post and exactly right. I'll bet you smoke a pipe, don't you
    doctor?
     
    Geoff, Dec 26, 2013
    #20
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.