views:

487

answers:

3

So it's the third week of my life in C and I've been given the task of writing a program which takes in upto 100words which are upto 80characters long, calculates the average word length of the inputs, prints words larger than the average, and finally prints the average word size. EDIT: We also have to use an emalloc, a recursive output method, and free all used memory.

Success! ... or so I thought.

I wrote the following within Eclipse which uses gcc -E -P -v -dD as its compilation arguments, and I get no runtime errors whilst running the program using the provided testcases.

Now I have completed the code I have to reproduce it during a 30min practical. We are told we have to use a text editor and that gcc -W -Wall -ansi -pedantic will have to be used as the compilation arguments, but if I use these arguments instead it means my program always exits with `Bus error'

EDIT: Fixed and Formatted

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

#define MAXIMUM_STRING_LENGTH 80
#define MAXIMUM_ARRAY_LENGTH 100

void* memory_allocation(size_t sizeof_memory_required) {

    void* free_memory_pointer = malloc(sizeof_memory_required);

    if (free_memory_pointer/*exists*/) {
     return free_memory_pointer;
    } else {
     fprintf(stderr, "*** MEMORY ALLOCATION FAILURE ***\n");
     exit(EXIT_FAILURE);
    }
}

void print_larger_than_average_strings(char** string_pointers, int i, const double AVERAGE_STRING_LENGTH) {

    if (string_pointers[i]/*exist*/) {
     if (strlen(string_pointers[i]) > AVERAGE_STRING_LENGTH) {
      printf("%s\n", string_pointers[i]);
     }
     print_larger_than_average_strings(string_pointers, ++i, AVERAGE_STRING_LENGTH);
    } else {
     fprintf(stderr, "%.2f\n", AVERAGE_STRING_LENGTH);
    }
}

int main(void) {

    int string_count = 0;
    char string[MAXIMUM_STRING_LENGTH];
    char* string_pointer[MAXIMUM_ARRAY_LENGTH];
    int i;
    double character_count;

    while ((string_count < MAXIMUM_ARRAY_LENGTH) && (1 == scanf("%79s", string))) {
     string_pointer[string_count] = memory_allocation(sizeof string_pointer[0][0] * (strlen(string) + 1));
     strcpy(string_pointer[string_count++], string);
    }
    string_pointer[string_count] = NULL;
    for (i = 0; i < string_count; i++) {
     character_count += strlen(string_pointer[i]);
    }
    if (string_count/*exists*/) {
     print_larger_than_average_strings(string_pointer, 0, character_count / string_count);
     for (i = 0; i < string_count; i++) {
      free(string_pointer[i]);
     }
    }
    return EXIT_SUCCESS;
}
+1  A: 

There is no terminating condition in your recursive function named o. You have to stop recursion somehow.

Try this code without recursion:

void* AllocateMemory(size_t s)
{
    void* mp = malloc(s);
    if(mp)
     return mp;
    else 
    {
        fprintf(stderr,"MALLOC FAILED!\n");
        exit(1);
    }
}

void PrintLargerWords(const char* wpp, double av)
{
    if(wpp)
    {
        if(strlen(wpp) > av)
      printf("%s\n",wpp);
    }
    else 
     fprintf(stderr,"%.2f\n",av);
}

int main(void)
{
    char w[80];
    char* wp[100];
    int wc=0;
    double cc=0;
    while(wc<5 && 1==scanf("%79s",w))
    {
        int length = strlen(w);
     wp[wc]= (char*)AllocateMemory((length+1) * sizeof(wp[0][0]));
        strcpy(wp[wc++],w);
     cc += length;
    }
    for(int j = 0; j<wc; j++)
    {
     PrintLargerWords(wp[j],cc/wc);
    }
    for(int i=0; i<wc; i++)
     free(wp[i]);
    return 0;
}

I have not touched most of your code. Do the proper error-checking as well.

Aamir
+3  A: 

The termination condition of your recursive function o is broken. The array of strings is not null terminated, why should it be? Why not just use a simple for there, instead of the recursive function? Something like:

for(i=0;i<wc;i++) {
      if(strlen(wpp[i])>av)
         printf("%s\n",wpp[i]); 
 }

Here is how I found out, using gdb:

t@c:~/tmp$ gcc -g -Wall -W -ansi -pedantic  test.c                      
t@c:~/tmp$ gdb ./a.out 
GNU gdb 6.8-debian                  
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.           
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-linux-gnu"...
(gdb) r
Starting program: /home/tudor/tmp/a.out
sdafasd
sadsa
sasaaaaaaaa
saaaaaaaaaa
asss
sasaaaaaaaa
saaaaaaaaaa

Program received signal SIGSEGV, Segmentation fault.
0xb7eba1e3 in strlen () from /lib/i686/cmov/libc.so.6
(gdb) bt
#0  0xb7eba1e3 in strlen () from /lib/i686/cmov/libc.so.6
#1  0x08048618 in o (wpp=0xbffad51c, av=7.5999999999999996, i=5) at test.c:13
#2  0x08048662 in o (wpp=0xbffad51c, av=7.5999999999999996, i=5) at test.c:14
#3  0x08048662 in o (wpp=0xbffad51c, av=7.5999999999999996, i=4) at test.c:14
#4  0x08048662 in o (wpp=0xbffad51c, av=7.5999999999999996, i=3) at test.c:14
#5  0x08048662 in o (wpp=0xbffad51c, av=7.5999999999999996, i=2) at test.c:14
#6  0x08048662 in o (wpp=0xbffad51c, av=7.5999999999999996, i=1) at test.c:14
#7  0x0804876c in main () at test.c:25
(gdb) frame 1
#1  0x08048618 in o (wpp=0xbffad51c, av=7.5999999999999996, i=5) at test.c:13
13              if(strlen(wpp[i])>av)printf("%s\n",wpp[i]);
(gdb) p i
$1 = 5
(gdb) p wpp
$2 = (char **) 0xbffad51c
(gdb) p *wpp[i]
Cannot access memory at address 0x8
tsg
+3  A: 

You have wrong "stop condition" in recurrsive functiuon "o". It checks if(wpp[i]) but last element of this array is not initialized to NULL. You should: change "char *wp[100]" to "char *wp[101]; wp[100]=NULL;". Then it should work but it's still not a pretty piece of code..

Pawel Kolodziej
PS. you should laso have wp[wc]=NULL;just before for(...) in main ();
Pawel Kolodziej