Need advice on C program 
Author Message
 Need advice on C program

Problem: Given an input string of the form (example)
"A=sdf;B=ddf;T1=22;L1=3;T2=5;L2=4;T3=5;T4=7;L4=4"
I need to extract the values of T1 = 22, L1 = 3, T2 = 5, L2 = 4, T3 =
5, L3 = (default), T4 = 7, L4 = 4
L3 defaults to a default value as it is not present in the string.
The program needs to parse the string to find the Ti, Li pairs. If Li
is not present for a Ti, it defaults to a default value. The parsing
stops when we cannot find Ti in the string. In the above example,
parsing stops at T5. Also it is guaranteed that there are no spaces in
the input string.
I wrote a small program to do this. It appears to work fine but I
would be grateful if someone could point out flaws/suggestions etc to
make this efficient and robust. I am not very good at C programming.

#include <stdio.h>
#include <string.h>

int main(void)
{
  char *GetParamValue(char *, char *);
  char sURL[200], *result, TR[3], TL[3];
  int i;
  result = (char *)malloc(15*sizeof(char));
  printf("Enter the URL string: ");
  scanf("%s", sURL);
  i = 1;
  sprintf(TR, "T%d", i);
  sprintf(TL, "L%d", i);
  printf("TR = %s\n", TR);
  printf("TL = %s\n", TL);
  while(result = GetParamValue(TR, sURL))
  {
    printf("Trace region(%d) = %s\n", i, result);
    if(result = GetParamValue(TL, sURL))
    {
      printf("Trace level = %s\n", result);
    }
    else
    {
      printf("Default trace level\n");
    }
    ++i;
    sprintf(TR, "T%d", i);
    sprintf(TL, "L%d", i);
    printf("TR = %s\n", TR);
    printf("TL = %s\n", TL);
  }

Quote:
}

char *GetParamValue(char *needle, char *haystack)
{
  int i;
  char value[15], *retVal, *t;
  char e[] = "=";
  retVal = (char *)malloc(20*sizeof(char));
  strcpy(value, needle);
  strcat(value, e);
  printf("The value being searched is = %s\n", value);
  t = strstr(haystack, value);
  if(t)
  {
    int j;
    i = 0;
    while(t[i] != '=')
      ++i;
    ++i;
    for(j = i; (t[i] != ';' && i < strlen(t)); i++)
      retVal[i-j] = t[i];
    retVal[i-j] = '\0';
    return(retVal);
  }
  else
  {
    printf("No luck\n");
    return(NULL);
  }
Quote:
}



Thu, 06 Jan 2005 15:12:29 GMT  
 Need advice on C program



Quote:
> Problem: Given an input string of the form (example)
> "A=sdf;B=ddf;T1=22;L1=3;T2=5;L2=4;T3=5;T4=7;L4=4"
> I need to extract the values of T1 = 22, L1 = 3, T2 = 5, L2 = 4, T3 =
> 5, L3 = (default), T4 = 7, L4 = 4
> L3 defaults to a default value as it is not present in the string.
> The program needs to parse the string to find the Ti, Li pairs. If Li
> is not present for a Ti, it defaults to a default value. The parsing
> stops when we cannot find Ti in the string. In the above example,
> parsing stops at T5. Also it is guaranteed that there are no spaces in
> the input string.
> I wrote a small program to do this. It appears to work fine but I
> would be grateful if someone could point out flaws/suggestions etc to
> make this efficient and robust. I am not very good at C programming.

Hi,
there may be other issues with your code, I made comments where I found
something:

#include <stdio.h>
#include <string.h>
/*you forgot to #include <stdlib.h>. Turn yor warning level up, the compiler
should have warned you about this*/
#include <stdlib.h>

int main(void)
{
  char *GetParamValue(char *, char *);
  char sURL[200], *result, TR[3], TL[3];
  int i;

  /*result = (char *)malloc(15*sizeof(char));
  no cast necessary if stdlib.h is #included, sizeof(char) is guaranteed to
be 1,
  generally I'd write:*/
  result = malloc(15 * sizeof *result);
  printf("Enter the URL string: ");
  /*scanf("%s", sURL); IMO*/
  fgets(sURL, sizeof sURL, stdin);
  /*might be a better choice*/
  i = 1;
  sprintf(TR, "T%d", i);
  sprintf(TL, "L%d", i);
  printf("TR = %s\n", TR);
  printf("TL = %s\n", TL);
  /*while(result = GetParamValue(TR, sURL)) this causes a warning from my
compiler:
    "assignment within conditional expression", I'd write:*/
  while((result = GetParamValue(TR, sURL)) != NULL)
  {
    printf("Trace region(%d) = %s\n", i, result);
    /*if(result = GetParamValue(TL, sURL)) same here:*/
    if((result = GetParamValue(TL, sURL)) != NULL)
    {
      printf("Trace level = %s\n", result);
    }
    else
    {
      printf("Default trace level\n");
    }
    ++i;
    sprintf(TR, "T%d", i);
    sprintf(TL, "L%d", i);
    printf("TR = %s\n", TR);
    printf("TL = %s\n", TL);
  }

Quote:
}

char *GetParamValue(char *needle, char *haystack)
{
  /*int i; this again causes a warning in the for() statement below, because
strlen returns
  a size_t which is unsigned. I'd make this:*/
  size_t i;
  char value[15], *retVal, *t;
  char e[] = "=";
  /*retVal = (char *)malloc(20*sizeof(char)); see my comment to your forst
malloc()*/
  retVal = malloc(20 * sizeof *retVal);
  strcpy(value, needle);
  strcat(value, e);
  printf("The value being searched is = %s\n", value);
  t = strstr(haystack, value);
  if(t)
  {
    int j;
    i = 0;
    while(t[i] != '=')
      ++i;
    ++i;
    for(j = i; (t[i] != ';' && i < strlen(t)); i++)
      retVal[i-j] = t[i];
    retVal[i-j] = '\0';
    return(retVal);
  }
  else
  {
    printf("No luck\n");
    return(NULL);
  }
Quote:
}



Thu, 06 Jan 2005 15:58:59 GMT  
 Need advice on C program

Quote:

> Problem: Given an input string of the form (example)
> "A=sdf;B=ddf;T1=22;L1=3;T2=5;L2=4;T3=5;T4=7;L4=4"
> I need to extract the values of T1 = 22, L1 = 3, T2 = 5, L2 = 4, T3 =
> 5, L3 = (default), T4 = 7, L4 = 4
> L3 defaults to a default value as it is not present in the string.
> The program needs to parse the string to find the Ti, Li pairs. If Li
> is not present for a Ti, it defaults to a default value. The parsing
> stops when we cannot find Ti in the string. In the above example,
> parsing stops at T5. Also it is guaranteed that there are no spaces in
> the input string.
> I wrote a small program to do this. It appears to work fine but I
> would be grateful if someone could point out flaws/suggestions etc to
> make this efficient and robust. I am not very good at C programming.

> #include <stdio.h>
> #include <string.h>

You forgot to include <stdlib.h>, for malloc's prototype.

Quote:

> int main(void)
> {
>   char *GetParamValue(char *, char *);

This is fine, but you might find it more useful to place the prototype
at file scope.

Quote:
>   char sURL[200], *result, TR[3], TL[3];
>   int i;
>   result = (char *)malloc(15*sizeof(char));

Because you forgot to include a prototype for malloc, the compiler is
obliged to assume that malloc returns int, whereas in fact it returns a
pointer. (Compilers are free to store ints and pointers in different
registers, so your return value might be fetched from the wrong place!)

Were it not for your (unnecessary) cast, the compiler would have warned
you that you were assigning what it thinks is an int, into a pointer
object ("result"). Since you introduced the cast, however, the compiler
will introduce object code for performing the necessary conversion from
int to pointer. This may result in a loss of information.

Also, sizeof(char) is guaranteed to be 1.

Good style: (a) remember to #include <stdlib.h> and (b) replace your
line with the following:

  result = malloc(15 * sizeof *result);

(This style works where result is a pointer to any type except void.)

(c) Don't forget to check that the allocation succeeded. (If malloc
returns NULL, the allocation did *not* succeed, and you should take
appropriate steps.)

Quote:
>   printf("Enter the URL string: ");

If there's no newline character '\n' at the end of the prompt, the
prompt may not appear before the program blocks for input. You can take
steps to prevent this problem by flushing the standard output stream:

  fflush(stdout);

Quote:
>   scanf("%s", sURL);

Should the user type 200 or more characters, scanf will cheerfully
overwrite memory you don't own, invoking undefined behaviour. I prefer
fgets() for this kind of input.

Quote:
>   i = 1;
>   sprintf(TR, "T%d", i);
>   sprintf(TL, "L%d", i);
>   printf("TR = %s\n", TR);
>   printf("TL = %s\n", TL);
>   while(result = GetParamValue(TR, sURL))

I don't mean to be rude, but it might be better for you to avoid tricks
like this until you have more experience in the language. After all, =
and == are sometimes easy to confuse. You might at least like to
consider this alternative:

    while((result = GetParamValue(TR, sURL)) != 0)

which makes your intention crystal-clear.

Of course, there is another problem here. Earlier, you used "result" to
catch a malloc return value. By overwriting that value here, you've
created a memory leak in your program.

<snip>

--

"Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
K&R answers, C books, etc: http://users.powernet.co.uk/eton



Thu, 06 Jan 2005 17:27:24 GMT  
 Need advice on C program


Quote:
>Problem: Given an input string of the form (example)
>"A=sdf;B=ddf;T1=22;L1=3;T2=5;L2=4;T3=5;T4=7;L4=4"
>I need to extract the values of T1 = 22, L1 = 3, T2 = 5, L2 = 4, T3 =
>5, L3 = (default), T4 = 7, L4 = 4
>L3 defaults to a default value as it is not present in the string.
>The program needs to parse the string to find the Ti, Li pairs. If Li
>is not present for a Ti, it defaults to a default value. The parsing
>stops when we cannot find Ti in the string. In the above example,
>parsing stops at T5. Also it is guaranteed that there are no spaces in
>the input string.
>I wrote a small program to do this. It appears to work fine but I
>would be grateful if someone could point out flaws/suggestions etc to
>make this efficient and robust. I am not very good at C programming.

>#include <stdio.h>
>#include <string.h>

>int main(void)
>{
>  char *GetParamValue(char *, char *);
>  char sURL[200], *result, TR[3], TL[3];
>  int i;
>  result = (char *)malloc(15*sizeof(char));
>  printf("Enter the URL string: ");
>  scanf("%s", sURL);
>  i = 1;
>  sprintf(TR, "T%d", i);
>  sprintf(TL, "L%d", i);
>  printf("TR = %s\n", TR);
>  printf("TL = %s\n", TL);
>  while(result = GetParamValue(TR, sURL))
>  {
>    printf("Trace region(%d) = %s\n", i, result);
>    if(result = GetParamValue(TL, sURL))
>    {
>      printf("Trace level = %s\n", result);
>    }
>    else
>    {
>      printf("Default trace level\n");
>    }
>    ++i;
>    sprintf(TR, "T%d", i);
>    sprintf(TL, "L%d", i);
>    printf("TR = %s\n", TR);
>    printf("TL = %s\n", TL);
>  }
>}
>char *GetParamValue(char *needle, char *haystack)
>{
>  int i;
>  char value[15], *retVal, *t;
>  char e[] = "=";
>  retVal = (char *)malloc(20*sizeof(char));
>  strcpy(value, needle);
>  strcat(value, e);
>  printf("The value being searched is = %s\n", value);
>  t = strstr(haystack, value);
>  if(t)
>  {
>    int j;
>    i = 0;
>    while(t[i] != '=')
>      ++i;
>    ++i;
>    for(j = i; (t[i] != ';' && i < strlen(t)); i++)
>      retVal[i-j] = t[i];

The only use if h is to compute i-j.  It might be clearer when you
revisit this program months later if you used j directly.  The for
statement may or may not compute the length of t each iteration,
depending on the optimization.  You can eliminate both problems with
        j = 0;
        while ( t[i] != ';' && t[i] != '\0')
                retVal[j++] = t[i++];

Quote:
>    retVal[i-j] = '\0';

And retVal[j+1] here also.

Quote:
>    return(retVal);
>  }
>  else
>  {
>    printf("No luck\n");
>    return(NULL);
>  }
>}

<<Remove the del for email>>


Fri, 07 Jan 2005 05:59:42 GMT  
 Need advice on C program

Quote:
> /*you forgot to #include <stdlib.h>. Turn yor warning level up, the compiler
> should have warned you about this*/

Which compiler does so? I'm just curious.

--

"LISP  is worth learning for  the profound enlightenment  experience
you will have when you finally get it; that experience will make you
a better programmer for the rest of your days."   -- Eric S. Raymond



Fri, 07 Jan 2005 17:58:12 GMT  
 Need advice on C program

Quote:


>> /*you forgot to #include <stdlib.h>. Turn yor warning level up, the compiler
>> should have warned you about this*/

> Which compiler does so? I'm just curious.

gcc does. at least with -Wall (which i usually compile my stuff with)
it will tell you "warning: implicit declaration of function `strcat'",
which should tell you <string.h> is missing.

--
Joost Kremers           http://baserv.uci.kun.nl/~jkremers
Ask 8 slackers how to do something, get 10 answers.
        -- sl in alt.os.linux.slackware



Fri, 07 Jan 2005 18:25:01 GMT  
 Need advice on C program

Quote:

> > /*you forgot to #include <stdlib.h>. Turn yor warning level up, the
compiler
> > should have warned you about this*/

> Which compiler does so? I'm just curious.

I'm not aware of any that warn about forgetting <stdlib.h> directly. But
there are many compilers that will issue a warning, even an error as an
option, if an applied function does not have a visible prototype.

Sort of a placebo for old malloc casters like me... ;)

--
Peter



Fri, 07 Jan 2005 18:36:51 GMT  
 Need advice on C program

Quote:


>>> /*you forgot to #include <stdlib.h>. Turn yor warning level up, the compiler
>>> should have warned you about this*/

>> Which compiler does so? I'm just curious.

> gcc does. at least with -Wall (which i usually compile my stuff with)
> it will tell you "warning: implicit declaration of function `strcat'",
> which should tell you <string.h> is missing.

Yes, this is similar to what you get from gcc when you don't
cast malloc and have no prototype in place. But the compiler
doesn't warn you explicitly about forgetting to include
stdlib.h because it cannot know what header you forgot, can it?
Well it could know that malloc should be declared in stdlib.h
and could warn you when you use malloc, OTOH most compilers (so
does gcc) have a compatibility mode to support old code (K&R)
and would probably expact malloc.h instead. This is not an
argument, its just that I don't know about any compiler that
warns explicitly about forgetting to include ... Yes it should
warn me about implicit declaration and about conversions when
there shouldn't be any, but it doesn't and probably cannot warn
about forgetting to include ... this is just a conclusion that the
programmer has to realize from other warnings.

--

"LISP  is worth learning for  the profound enlightenment  experience
you will have when you finally get it; that experience will make you
a better programmer for the rest of your days."   -- Eric S. Raymond



Fri, 07 Jan 2005 19:45:50 GMT  
 
 [ 8 post ] 

 Relevant Pages 

1. Reuse of cs files, namespace, arch advice pls

2. Help!!! Novice CS Advice

3. programming language advice needed

4. need programming advice

5. Need Help/Advice With This C Program...

6. New to programming...advice needed!

7. Programming advice needed

8. Program freezes with Sleep() - now i need some advice about a multi-threaded app

9. Internet programming advice needed

10. Advice needed: Alternative make programs

11. Needing advice on Wav programming

12. Newbie: separate big .cs file into small .cs files

 

 
Powered by phpBB® Forum Software