Re: String manipulation program not returning expected output


"Logan Lee" <Logan.W.Lee@xxxxxxxxxxxxxxxxxx> wrote in message
news:878x3wk8uq.fsf@xxxxxxxxxxxxxxxxxxxxx

Hi. I've written a small program to learn to write in C. But unfortunately
the output is all jumbled up and not nice.

....
I'm expecting the output

def

But I'm getting others with jumbled characters.

There are numerous issues, but the reason your getting "jumbled" characters
is due to the way you dynamically allocate space for file_content.


/* read_file.c
The whole point of this code is to read the entire content from a file
then arrange the data as a single string. */

#include <stdio.h>


I treated all three routines as one file and needed these includes for my
suggested changes:

#include <stdlib.h>
#include <string.h> /* for my changes */

char* returnArrayFromFile(char* file_name) {
// Try opening a file
FILE *text_file=fopen(file_name,"r");

There's non-obvious coding error here interacting with the rest of the
program. It has to due with the character(s) '\n'... I don't expect you to
understand this. Anyway, you'll want to use "rb" instead of "r". In fact,
you'll save yourself much grief if you just learn to avoid non-binary modes
as much as possible. So, use this:
FILE *text_file=fopen(file_name,"rb");


// Total number of characters in the file.
int m=0;
while(feof(text_file)==0) {
fgetc(text_file); m++;
}

The while loop is equivalent to using fseek() and ftell():
fseek(text_file,0L,SEEK_END);
m=ftell(text_file);

fclose(text_file);
fopen("text_file","r");

The fopen line is erroneous. You meant file_name instead of "text_file".
Again, you'll need "rb" instead of "r":
fopen(file_name,"rb");


char file_content[m];

The declaration of file_content has two errors. The first is that the scope
of the storage is limited to returnArrayFromFile(), i.e., you can't use the
storage outside of returnArrayFromFile()... You want to use malloc(). The
second error is an off by one. You need to allocate m+1 since you need to
ensure that the array in main(), called larger, has an additional character
so it can be nul terminated. You want this:
char *file_content=malloc(m+1);


int i=0;
while(i<m) {
fscanf(text_file, "%c", &file_content[i++]);
}


This while loop is equivalent to a call to fread(). one(1) in the following
is the size which is sizeof(char), i.e., one(1).
fread(file_content,1,m,text_file);

fclose(text_file);

return file_content;
}


/* substring.c
Return B from ABC */
char* getSubstring(char* larger, int a, int b) {
char* smaller=(char *)malloc(sizeof(char) * b);

There are two errors in the malloc of smaller. The first is that the cast
on malloc is unecessary, an obsolete coding style, and some say prevents
locating certain errors. The second is that it too is off by one. You'll
need an additional character for the nul. Also, the sizeof(char) is always
one(1). You want this:
char* smaller=malloc(b+1);

int i;
for(i=0; i<b; i++) {
if (larger[a+i]=='\0') {
break;
}
smaller[i]=larger[a+i];
}

There is a coding error in the loop. You are checking for nul, '\0', but
you never set a nul in larger or file_content... One doesn't exist in the
text file. I believe, but didn't thoroughly check, that the if()-break
should be after the smaller=larger statement to copy the nul, _if_ it had
been there... Anyway, the for loop can be reworked to the following which
nul terminates both larger and smaller.

larger[a+b]='\0';
strcpy(smaller,&larger[a]);

return smaller;
}

/* main.c */
#include "read_file.h"
#include "substring.h"

int main(int argc, char* argv[]) {
char* file_name=argv[1];
int a=atoi(argv[2]);
int b=atoi(argv[3]);
char* larger=returnArrayFromFile(file_name);
char* smaller=getSubstring(larger, a, b);
printf("%s", smaller);
}

/* Lastly, text_file */
abcdefghi

If I compile them with gcc *.c -o main and execute by


If you rework the C99 features to C90, declarations at the top of the
procedure, no C++ comments, no dynamic allocations, then you can increase
the level of warnings to detect problems:
gcc -Wall -ansi -pedantic *.c -o main

Anyway there are other issues you need to look into:
1) not checking that malloc returned space
2) missing return(0) or exit(EXIT_SUCCESS) for main()
3) strtod,strtol,strtoul have fewer side effects than atoi
4) assumed passed in parameters are the correct type


Rod Pemberton

.

Relevant Pages

  • Re: Returning a 2D array from a function
    ... int mainplease ... You should always check malloc for success. ... This creates a memory leak. ... All of your output is ascii characters. ...
    (comp.lang.c)
  • Re: Malloc code
    ... int CHARSPACING; //SPACES BETWEEN CHARACTERS ... also said that for your compiler, int is 8 bits. ... declaration of malloc from your compiler's malloc.h. ... elements beyond the first five are outside the allocated buffer. ...
    (microsoft.public.vc.language)
  • Re: How to convert Infix notation to postfix notation
    ... int intMaxLength) ... array of 15 characters, and you call this function with the limit 15 on ... int appendPolish(char *strPolish, char chrNext) ... give a message like "string too long". ...
    (comp.lang.c)
  • Re: Checking for a keypress on Linux ?????
    ... the short delay in kbhit() ... > int getch; ... > * returns the number of characters available to read. ... > static struct termios Otty, ...
    (comp.os.linux.development.apps)
  • Re: [PATCH] Fix console utf8 composing
    ... So, the result is that the result of the composing operation is still taken from the accent_table, and thus cannot be more than "unsigned char" allows. ... in UTF-8 mode, it is possible to generate characters from Latin-1 set by composing, and they are generated correctly. ... +static int use_unicode; ...
    (Linux-Kernel)
Loading