Re: Comments on my code?

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;
}
.

Relevant Pages

  • Re: Comments on my code?
    ... these are extremely old lecture notes. ... int main(int argc, char **argv) ... I would have written the above declaration as: ... This looks OK to me and works correctly, but the compiler produces ...
    (comp.lang.c)
  • Re: MFC dll and exe porting to 64-bit
    ... Why the obsolete 'char *' instead of the proper LPTSTR? ... We are no longer programming 16-bit windows in an 8-bit character world, ... string arguments should probably be LPCTSTR, that is, const parameters. ... declaration of the function itself, in context (meaning show the surrounding class ...
    (microsoft.public.vc.mfc)
  • Re: malloc warning gcc > 4.0
    ... > char* p; ... > warning: incompatible implicit declaration of built-in function 'malloc' ...
    (comp.lang.c.moderated)
  • Re: Banks and economy
    ... char cptr; ... I'm assuming that it's at file scope, because otherwise the array declaration wouldn't be allowed. ... In some cases, a tentative definition with no corresponding external definition ends up being treated as the actual definition, with an implicit initializer of 0. ...
    (comp.lang.c)
  • Re: Access one character in an array of characters
    ... enum Suit; ... Without seeing the exact declaration of ClubArray I cannot be certain, ... but you have probably declared it as a C-style string, ... confusion between a string and a vector of char. ...
    (alt.comp.lang.learn.c-cpp)