Comments on my code?

Discussion in 'C Programming' started by Platonic Solid, Sep 1, 2007.

  1. Hi-

    I am learning C from some old lecture notes, but they don't have
    solutions so I'd like some feedback on my code if anyone has the time.

    This is an exercise: "Write a program to trim any leading whitespace
    from a string and return a newly allocated buffer containing the trimmed
    string. Don't forget to handle errors."

    main(argc, argv)
    int argc; char **argv;
    {
    char *strtrim(), *r=0;
    puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    free(r);
    }

    /* trims initial whitespace */
    char *strtrim(s)
    char *s;
    {
    char *r, *malloc();
    for( ; isspace(*s); s++);
    r=malloc(strlen(s)+1);
    if(r)
    strcpy(r,s);
    return r;
    }

    This looks OK to me and works correctly, but the compiler produces some
    mysterious warnings about conflicting definitions that I don't really
    understand.


    --
    How come we never talk anymore?
     
    Platonic Solid, Sep 1, 2007
    #1
    1. Advertising

  2. Platonic Solid

    CBFalconer Guest

    Platonic Solid wrote:
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the
    > time.
    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the
    > trimmed string. Don't forget to handle errors."
    >
    > main(argc, argv)
    > int argc; char **argv;
    > {
    > char *strtrim(), *r=0;
    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    > free(r);
    >

    .... snip ...
    >
    > This looks OK to me and works correctly, but the compiler produces
    > some mysterious warnings about conflicting definitions that I
    > don't really understand.


    To start with, you are failing to use prototypes (using obsolete
    K&R 1 function headers), omitting necessary #includes, and using
    system reserved names. The latter includes anything of the form
    "strx..." where x is a lowercase letter.

    You can get a satisfactory approximation to the standard in N869 or
    N1124. You can find a suitable bzip2 compressed version of N869
    at:

    <http://cbfalconer.home.att.net/download/>

    --
    Chuck F (cbfalconer at maineline dot net)
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net>



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Sep 1, 2007
    #2
    1. Advertising

  3. Platonic Solid

    Eric Sosman Guest

    Platonic Solid wrote:
    > Hi-
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the time.
    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the trimmed
    > string. Don't forget to handle errors."
    >
    > main(argc, argv)
    > int argc; char **argv;
    > {
    > char *strtrim(), *r=0;
    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    > free(r);
    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;
    > {
    > char *r, *malloc();
    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > This looks OK to me and works correctly, but the compiler produces some
    > mysterious warnings about conflicting definitions that I don't really
    > understand.


    The mysterious warning "Mene mene tekel upharsin" means
    it's time to put your affairs in order; God is about to smite
    you down.

    The mysterious warning "Beware the Ides of March" means
    it's time to put your affairs in order; Brutus et al. are
    about to smite you down.

    You can find examples of other mysterious warnings in
    the comp.lang.c Frequently Asked Questions (FAQ) list. In
    particular, I commend Question 1.25 to your attention.

    (Your code has other issues, too, that you will learn
    about as you go along. Question 1.29 is worth a look.)

    --
    Eric Sosman
    lid
     
    Eric Sosman, Sep 1, 2007
    #3
  4. Platonic Solid

    jacob navia Guest

    Platonic Solid wrote:
    > Hi-
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the time.
    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the trimmed
    > string. Don't forget to handle errors."
    >
    > main(argc, argv)
    > int argc; char **argv;
    > {
    > char *strtrim(), *r=0;
    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    > free(r);
    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;
    > {
    > char *r, *malloc();
    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > This looks OK to me and works correctly, but the compiler produces some
    > mysterious warnings about conflicting definitions that I don't really
    > understand.
    >
    >
    > --
    > How come we never talk anymore?


    You should not declare malloc like that.
    #include <stdlib.h>
    and never define your own definitions for library functions.

    You need
    #include <string.h>
    for strlen's prototype.

    And:
    > char *strtrim(s)
    > char *s;


    This is old fashioned C. Better is:
    > char *strtrim(char *s)


    P.S. What happens when s is NULL?

    jacob
     
    jacob navia, Sep 1, 2007
    #4
  5. Platonic Solid wrote:
    > Hi-
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the time.
    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the trimmed
    > string. Don't forget to handle errors."
    >
    > main(argc, argv)
    > int argc; char **argv;
    > {
    > char *strtrim(), *r=0;
    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    > free(r);
    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;
    > {
    > char *r, *malloc();
    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > This looks OK to me and works correctly, but the compiler produces some
    > mysterious warnings about conflicting definitions that I don't really
    > understand.


    The program you wrote would be a correct way to write code a long time ago,
    so if your lecture notes were really old, then well done. It's not a right
    way to do things now, though. Try this version, and see if you understand
    the changes I made:

    #include <stdio.h> /* include the headers that declare the library */
    #include <stdlib.h> /* functions and objects you are using. you should */
    #include <string.h> /* not declare them in your own program, especially*/
    #include <ctype.h> /* the wrong way; that is what caused your warning */

    /*
    * a forward declaration for your function should not be local to
    * another function. also, you can specify the types of the parameters
    * in the declaration as well as the definition. doing so allows the
    * compiler to check and automatically convert the values you will be
    * passing to the function.
    *
    * I renamed the function because technically, strtrim isn't a valid name
    * for it, but the reasons why probably won't be of interest to you at
    * this stage.
    */
    char *trimstr(char *);

    /*
    * the types of the parameters should be specified in the parameter list,
    * not between the ) and {. it's still valid, but has the same problems
    * as not specifying the parameter types in the function declaration.
    * and you should specify the return type, even if it's int.
    */
    int main(int argc, char **argv) {
    /*
    * for readability reasons, I would recommend against your use of ?:,
    * and use if statements instead.
    */
    if (argc > 1) {
    char *r = trimstr(argv[1]);
    if (!r) {
    /* error messages should normally go to stderr, not stdout */
    fputs("Unspecified error\n", stderr);
    } else {
    puts(r);
    free(r);
    }
    }
    /* main returns int (even in your original version), so return an int */
    return 0;
    }

    char *trimstr(char *s) {
    char *r;
    /* for readability, you can rewrite your for loop */
    while (isspace(*s))
    s++;
    /*
    * malloc doesn't need to be declared manually here, with <stdlib.h>
    * included. and malloc doesn't return 'char *' anymore, but 'void *'
    * void is probably not covered in your lecture notes, but in the
    * context of pointers, it means a pointer to any object, but you
    * don't have the object's type.
    */
    r = malloc(strlen(s)+1);
    if(r)
    strcpy(r,s);
    return r;
    }
     
    Harald van =?UTF-8?B?RMSzaw==?=, Sep 1, 2007
    #5
  6. On 1 Sep 2007 at 12:44, Eric Sosman wrote:
    > The mysterious warning "Mene mene tekel upharsin" means
    > it's time to put your affairs in order; God is about to smite
    > you down.
    >
    > The mysterious warning "Beware the Ides of March" means
    > it's time to put your affairs in order; Brutus et al. are
    > about to smite you down.


    Not sure I understand the relevance of these illusions!

    > You can find examples of other mysterious warnings in
    > the comp.lang.c Frequently Asked Questions (FAQ) list. In
    > particular, I commend Question 1.25 to your attention.


    OK, so that explains why strcpy produces a warning as it returns char *
    and not int. But the compiler also complains about malloc (I
    specifically supply the return type for that) and strlen, which returns
    int so should be OK by Q 1.25.

    Thanks for the comments.

    --
    How come we never talk anymore?
     
    Platonic Solid, Sep 1, 2007
    #6
  7. On 1 Sep 2007 at 12:53, jacob navia wrote:
    > P.S. What happens when s is NULL?


    If you look carefully I check argc>1, so it never gets called with s
    null.

    --
    How come we never talk anymore?
     
    Platonic Solid, Sep 1, 2007
    #7
  8. Platonic Solid

    Army1987 Guest

    On Sat, 01 Sep 2007 14:01:58 +0200, Platonic Solid wrote:

    > Hi-
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the time.
    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the trimmed
    > string. Don't forget to handle errors."
    >
    > main(argc, argv)
    > int argc; char **argv;

    In modern C this is usually written as
    int main(int argc, char *argv[])
    But the old form continues to work, even it "looks strange".
    > {
    > char *strtrim(), *r=0;

    Better put function declaration at file scope, and use NULL
    defined in <stddef.h> and other headers instead of 0. Of course
    your form will work, too, but it is "ugly".

    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");

    To somebody which doesn't remember all the precedence rules by
    heart, this could be confusing. If you *really* think that if/else
    is evil (it isn't, at least it'd allow you to write the error
    message to stderr), at least put parentheses around the first
    operand of ?: ...
    <nitpick pedantry="high">Nowadays puts() takes a const char *,
    but you're passing it a char *, because there is no prototype in
    scope.</nitpick> Even if it is overwhelmingly likely to work,
    usually one use the prototype for puts() found in <stdio.h>.

    > free(r);
    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;
    > {
    > char *r, *malloc();

    In modern C malloc returns a void*, not a char*. Also the argument
    is a size_t, see FAQ 7.15.

    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);

    strlen returns a size_t, but here you're declaring it as returning
    an int.
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > This looks OK to me and works correctly, but the compiler produces some
    > mysterious warnings about conflicting definitions that I don't really
    > understand.

    See FAQ 11.29a for compatibility problem between old and new C.
    --
    Army1987 (Replace "NOSPAM" with "email")
    No-one ever won a game by resigning. -- S. Tartakower
     
    Army1987, Sep 1, 2007
    #8
  9. Platonic Solid

    jacob navia Guest

    Platonic Solid wrote:
    > On 1 Sep 2007 at 12:53, jacob navia wrote:
    >> P.S. What happens when s is NULL?

    >
    > If you look carefully I check argc>1, so it never gets called with s
    > null.
    >
    > --
    > How come we never talk anymore?


    Sure, but is strtrim only for this particular call?
    In that case a function is not necessary!
     
    jacob navia, Sep 1, 2007
    #9
  10. On 2007-09-01 13:26, jacob navia <> wrote:
    > Platonic Solid wrote:
    >> On 1 Sep 2007 at 12:53, jacob navia wrote:
    >>> P.S. What happens when s is NULL?

    >>
    >> If you look carefully I check argc>1, so it never gets called with s
    >> null.

    >
    > Sure, but is strtrim only for this particular call?
    > In that case a function is not necessary!


    What happens when you call strlen(NULL)?

    hp


    --
    _ | Peter J. Holzer | I know I'd be respectful of a pirate
    |_|_) | Sysadmin WSR | with an emu on his shoulder.
    | | | |
    __/ | http://www.hjp.at/ | -- Sam in "Freefall"
     
    Peter J. Holzer, Sep 1, 2007
    #10
  11. On 2007-09-01 12:59, Harald van Dijk <> wrote:
    > Platonic Solid wrote:
    >> I am learning C from some old lecture notes, but they don't have
    >> solutions so I'd like some feedback on my code if anyone has the time.


    [ lots of good advice snipped]


    > char *trimstr(char *s) {
    > char *r;
    > /* for readability, you can rewrite your for loop */
    > while (isspace(*s))
    > s++;


    isspace expects an argument in the range [EOF, 0 .. UCHAR_MAX]. But
    *s will be in the range [CHAR_MIN .. CHAR_MAX], so isspace may be called
    with an illegal value, resulting in undefined behaviour.

    You must get the value into the correct range first:

    while (isspace((unsigned char)*s))
    s++;

    hp

    --
    _ | Peter J. Holzer | I know I'd be respectful of a pirate
    |_|_) | Sysadmin WSR | with an emu on his shoulder.
    | | | |
    __/ | http://www.hjp.at/ | -- Sam in "Freefall"
     
    Peter J. Holzer, Sep 1, 2007
    #11
  12. Platonic Solid wrote:

    > Hi-
    >
    > I am learning C from some old lecture notes, but they don't have
    > solutions so I'd like some feedback on my code if anyone has the time.


    Judging from your code, these are extremely old lecture notes. If there
    is a date on them, I would guess it to be somewhere in the 1980's.
    Since that time, C has evolved a bit. Not too much but enough that it
    would be worthwhile to have a more recent book to accompany these
    lecture notes.
    I would suggest you get a copy of "The C Programming Language", second
    edition, by Kernighan and Ritchie (often referred to as K&R2)

    >
    > This is an exercise: "Write a program to trim any leading whitespace
    > from a string and return a newly allocated buffer containing the
    > trimmed string. Don't forget to handle errors."
    >

    To use library functions, you should #include the appropriate headers.
    For this program, you need

    #include <stdio.h> /* for puts() */
    #include <stdlib.h> /* for malloc()/free() */
    #include <ctype.h> /* for isspace() */
    #include <string.h> /* for strcpy() */

    > main(argc, argv)
    > int argc; char **argv;


    This style of declaring parameters is considered to be outdated.
    It is preferred that you write it now like this:
    int main(int argc, char **argv)

    > {
    > char *strtrim(), *r=0;


    It is not wrong to declare a function at this point, but it is not
    considered to be good style.
    It is recommended that you declare functions at file scope, and that you
    give a prototype for the function.
    The prototype for strtrim() looks like
    char *strtrim(char*);
    The advantage of a prototype is that the compiler can verify (and
    complain!) that you call the function in the correct way.

    As a more personal style issue, I prefer to have one item per
    declaration. I would have written the above declaration as:
    char *strtrim(char*);
    char *r;
    I also might have used a more descriptive name for r.

    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");


    C allows you to write extremely complex expressions, but usually it is
    best to avoid using that freedom. (exception: IOCCC entries)
    The statement above is right at the limit of how complex you should make
    it (and some would argue it is over the limit).

    > free(r);


    As main() returns a success/failure code to the OS, you should make sure
    that it is a sensible code:
    return 0; /* indicates success. Other code are EXIT_SUCCESS and
    EXIT_FAILURE */

    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;


    Same comment as with main(): you should place the argument type also
    within the parentheses.
    char *strtrim(char *s)
    > {
    > char *r, *malloc();


    You should not write a declaration for library functions, like malloc().
    Instead you should #include the required header (see above).

    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > This looks OK to me and works correctly, but the compiler produces
    > some mysterious warnings about conflicting definitions that I don't
    > really understand.


    Those warnings are probably due to the required headers that you forgot
    to include.
    Modern compilers will complain if the encounter a call to a function
    that they have not yet seen before (like your calls to puts, isspace
    and strcpy). The complaints are because the compiler has to make some
    assumptions that are likely to be wrong.

    BTW, if you have questions about compiler messages, it works so much
    better if you post the compiler output (along with the code you fed
    into the compiler). Most of us will know immediately what it means,
    because we have seen the message before.

    Bart v Ingen Schenau
    --
    a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
    c.l.c FAQ: http://www.eskimo.com/~scs/C-faq/top.html
    c.l.c++ FAQ: http://www.parashift.com/c -faq-lite/
     
    Bart van Ingen Schenau, Sep 1, 2007
    #12
  13. Peter J. Holzer wrote:
    > On 2007-09-01 12:59, Harald van Dijk <> wrote:
    >> Platonic Solid wrote:
    >>> I am learning C from some old lecture notes, but they don't have
    >>> solutions so I'd like some feedback on my code if anyone has the time.

    >
    > [ lots of good advice snipped]
    >
    >
    >> char *trimstr(char *s) {
    >> char *r;
    >> /* for readability, you can rewrite your for loop */
    >> while (isspace(*s))
    >> s++;

    >
    > isspace expects an argument in the range [EOF, 0 .. UCHAR_MAX]. But
    > *s will be in the range [CHAR_MIN .. CHAR_MAX], so isspace may be called
    > with an illegal value, resulting in undefined behaviour.
    >
    > You must get the value into the correct range first:
    >
    > while (isspace((unsigned char)*s))
    > s++;


    Thanks, I forgot about that. It would be nice if I could get my system to
    warn about passing a char; it's not the first time I've left out the cast.
     
    Harald van =?UTF-8?B?RMSzaw==?=, Sep 1, 2007
    #13
  14. Platonic Solid

    CBFalconer Guest

    Platonic Solid wrote:
    >

    .... snip ...
    >
    > OK, so that explains why strcpy produces a warning as it returns
    > char * and not int. But the compiler also complains about malloc
    > (I specifically supply the return type for that) and strlen, which
    > returns int so should be OK by Q 1.25.


    You didn't give code, so we can't make suggestions. If you have
    problems with malloc you are probably a) casting the return or b)
    failing to include <stdlib.h> or c) capturing the return value in
    other than a pointer or d) using a C++ compiler.

    --
    Chuck F (cbfalconer at maineline dot net)
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net>



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Sep 1, 2007
    #14
  15. On 1 Sep 2007 at 16:42, CBFalconer wrote:
    > Platonic Solid wrote:
    >>

    > ... snip ...
    >>
    >> OK, so that explains why strcpy produces a warning as it returns
    >> char * and not int. But the compiler also complains about malloc
    >> (I specifically supply the return type for that) and strlen, which
    >> returns int so should be OK by Q 1.25.

    >
    > You didn't give code, so we can't make suggestions.


    On the contrary, I posted my code, but here it is again:

    main(argc, argv)
    int argc; char **argv;
    {
    char *strtrim(), *r=0;
    puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    free(r);
    }

    /* trims initial whitespace */
    char *strtrim(s)
    char *s;
    {
    char *r, *malloc(), *strcpy();
    for( ; isspace(*s); s++);
    r=malloc(strlen(s)+1);
    if(r)
    strcpy(r,s);
    return r;
    }

    (I've made one fix, by putting in the correct return type for strcpy)


    > If you have problems with malloc you are probably a) casting the
    > return or b) failing to include <stdlib.h> or c) capturing the return
    > value in other than a pointer or d) using a C++ compiler.


    On the contrary, I've declared malloc correctly and store its return
    value in a char *.

    I'm not sure what to do about people's advice that some of the syntax in
    the lecture notes is now old-fashioned - as it still seems to compile
    just fine, I'm tempted to stick with what I know.

    > --
    > Chuck F (cbfalconer at maineline dot net)
    > Available for consulting/temporary embedded and systems.
    > <http://cbfalconer.home.att.net>
    >
    >
    >


    --
    How come we never talk anymore?
     
    Platonic Solid, Sep 1, 2007
    #15
  16. Platonic Solid wrote:
    > char *r, *malloc(), *strcpy();
    >
    > On the contrary, I've declared malloc correctly and store its return
    > value in a char *.


    No, you haven't declared malloc correctly.
     
    Harald van =?UTF-8?B?RMSzaw==?=, Sep 1, 2007
    #16
  17. Harald van Dijk <> writes:
    > Platonic Solid wrote:
    >> char *r, *malloc(), *strcpy();
    >>
    >> On the contrary, I've declared malloc correctly and store its return
    >> value in a char *.

    >
    > No, you haven't declared malloc correctly.


    Specifically, malloc returns void*, not char*, and it takes an
    argument of type size_t. Since you didn't declare the argument type,
    something as simple as malloc(42) invokes undefined behavior, since
    you're passing an argument of type int.

    If you had declared it correctly as
    void *malloc(size_t);
    then a call like malloc(42) would implicitly convert the argument to
    size_t -- and a call with a non-numeric argument would trigger an
    error message.

    But the best way to provide the correct declaration is not to
    manually re-write it yourself. It's to provide
    #include <stdlib.h>
    at the top of your source file. There's simply no good reason not to
    do it that way.

    The more correct information you can provide for the compiler, the
    more it can help you. A lot of the facilities for this, particularly
    function prototypes, were introduced with the ANSI C standard in 1989.

    Prototypes were introduced into C 18 years ago. There are no longer
    any significant compilers still being used that don't support them, so
    there's no good reason not to use them, along with a number of other
    features introduced by ANSI. (The latest ISO C standard, C99, hasn't
    caught on as well, so there are still good reasons to avoid
    C99-specific features.)

    Your code is archaic. You can still write working archaic code if
    you're careful enough, but you really should learn the language as it
    is today. Reading K&R2 is probably the best way to do this.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Sep 1, 2007
    #17
  18. Platonic Solid

    Army1987 Guest

    On Sat, 01 Sep 2007 20:35:10 +0200, Platonic Solid wrote:

    > On the contrary, I've declared malloc correctly and store its return
    > value in a char *.
    >
    > I'm not sure what to do about people's advice that some of the syntax in
    > the lecture notes is now old-fashioned - as it still seems to compile
    > just fine, I'm tempted to stick with what I know.


    But, in ANSI/ISO C, which has been around since 1989 and radically
    revised in 1999 (even though this last version isn't widely
    implemented yet), malloc() returns a void *, not a char * anymore.
    It is very likely to work, since char* and void* are required to
    have the same alignment etc., but probably a sufficiently deep
    level of language lawyering would show that the behavior is
    undefined by the International Standard.

    Also, nowadays it is common to declare library functions with the
    prototypes in the corresponding headers. For example, stdlib.h
    contains a correct prototype for malloc(), and you can add it to
    your code with the preprocessing directive
    #include <stdlib.h>
    which gets replaced with the contents of that file.
    In C99 you *always* must declare functions, even if they return an
    int, and doing so is a very good idea in C89, too.
    --
    Army1987 (Replace "NOSPAM" with "email")
    No-one ever won a game by resigning. -- S. Tartakower
     
    Army1987, Sep 1, 2007
    #18
  19. Platonic Solid

    Guest

    On Sep 1, 8:22 pm, Keith Thompson <> wrote:
    > Harald van D k <> writes:
    >
    > > Platonic Solid wrote:
    > >> char *r, *malloc(), *strcpy();

    >
    > >> On the contrary, I've declared malloc correctly and store its return
    > >> value in a char *.

    >
    > > No, you haven't declared malloc correctly.

    >
    > Specifically, malloc returns void*, not char*, and it takes an
    > argument of type size_t. Since you didn't declare the argument type,
    > something as simple as malloc(42) invokes undefined behavior, since
    > you're passing an argument of type int.
    >
    > If you had declared it correctly as
    > void *malloc(size_t);
    > then a call like malloc(42) would implicitly convert the argument to
    > size_t -- and a call with a non-numeric argument would trigger an
    > error message.


    Am I right in thinking that first standard promotions would apply, and
    only then would the result be converted to size_t?

    > But the best way to provide the correct declaration is not to
    > manually re-write it yourself. It's to provide
    > #include <stdlib.h>
    > at the top of your source file. There's simply no good reason not to
    > do it that way.
    >
    > The more correct information you can provide for the compiler, the
    > more it can help you. A lot of the facilities for this, particularly
    > function prototypes, were introduced with the ANSI C standard in 1989.
    >
    > Prototypes were introduced into C 18 years ago. There are no longer
    > any significant compilers still being used that don't support them, so
    > there's no good reason not to use them, along with a number of other
    > features introduced by ANSI. (The latest ISO C standard, C99, hasn't
    > caught on as well, so there are still good reasons to avoid
    > C99-specific features.)
    >
    > Your code is archaic. You can still write working archaic code if
    > you're careful enough, but you really should learn the language as it
    > is today. Reading K&R2 is probably the best way to do this.
    >
    > --
    > Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    > San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    > "We must do something. This is something. Therefore, we must do this."
    > -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    , Sep 1, 2007
    #19
  20. Platonic Solid

    Eric Sosman Guest

    Platonic Solid wrote:
    > On 1 Sep 2007 at 16:42, CBFalconer wrote:
    >> Platonic Solid wrote:
    >> ... snip ...
    >>> OK, so that explains why strcpy produces a warning as it returns
    >>> char * and not int. But the compiler also complains about malloc
    >>> (I specifically supply the return type for that) and strlen, which
    >>> returns int so should be OK by Q 1.25.

    >> You didn't give code, so we can't make suggestions.

    >
    > On the contrary, I posted my code, but here it is again:
    >
    > main(argc, argv)
    > int argc; char **argv;
    > {
    > char *strtrim(), *r=0;
    > puts(argc>1 && (r=strtrim(*++argv)) ? r : "Unspecified error");
    > free(r);
    > }
    >
    > /* trims initial whitespace */
    > char *strtrim(s)
    > char *s;
    > {
    > char *r, *malloc(), *strcpy();
    > for( ; isspace(*s); s++);
    > r=malloc(strlen(s)+1);
    > if(r)
    > strcpy(r,s);
    > return r;
    > }
    >
    > (I've made one fix, by putting in the correct return type for strcpy)


    Yabbut, that's the wrong way to do it, and it still
    doesn't fix the incorrect declaration of strlen(). The
    right fix (hint: Have you learned about the #include
    directive yet?) would take care of both problems, and
    would enable the compiler to catch errors like strcpy(x)
    or strlen(x, y), too.

    >> If you have problems with malloc you are probably a) casting the
    >> return or b) failing to include <stdlib.h> or c) capturing the return
    >> value in other than a pointer or d) using a C++ compiler.

    >
    > On the contrary, I've declared malloc correctly and store its return
    > value in a char *.


    You have declared malloc *in*correctly. The cure is
    similar to that for strlen() et al.

    > I'm not sure what to do about people's advice that some of the syntax in
    > the lecture notes is now old-fashioned - as it still seems to compile
    > just fine, I'm tempted to stick with what I know.


    I bet your 1955 Studebaker runs just fine without those
    pesky seat belts, air bags, anti-lock brakes, and emission-
    control devices, too.

    --
    Eric Sosman
    lid
     
    Eric Sosman, Sep 1, 2007
    #20
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. learningjava
    Replies:
    8
    Views:
    495
    John C. Bollinger
    Dec 12, 2003
  2. Replies:
    0
    Views:
    1,177
  3. Monk
    Replies:
    10
    Views:
    1,538
    Michael Wojcik
    Apr 20, 2005
  4. Replies:
    4
    Views:
    648
    Dr John Stockton
    Jun 3, 2006
  5. Mclaren Fan
    Replies:
    2
    Views:
    671
    Richard Cornford
    Nov 8, 2011
Loading...

Share This Page