deleting last char in C

Question

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define ROZ 100
char *deleteLastChar(char* s)
{
    int rozmiar,i;
    char *tab1;

    rozmiar=strlen(s);
    tab1=(char*)malloc((rozmiar-1)*sizeof(int));
    for(i=0;i<(rozmiar-1); i++)
    {
        tab1[i]=s[i];
    }
    return tab1;
    free(s);
}



main()
{
    char tab1[ROZ], *tab2;
    int l,i,p,a;

    printf("podaj napis\n");

    fgets(tab1, 100, stdin);

    l=strlen(tab1);     
    tab2=malloc(l*sizeof(int));
    if (tab2==NULL)
        printf("blad");
    for(i=0;i<l;i++)
    {
        tab2 = deleteLastChar(tab2);
        p = strlen(tab2);
        for(a=0;a<p;a++)
        {
            printf("%c",tab2[a]);
        }
    }
}

Hi I'm trying to make a program which is deleting the last char from string over and over again until there is no more chars in it. It has to be done on dynamic arrays by shortening new arrray and rewrite previous array into it. Program is compiling without any errors, although it doesn't work as it suppouse to do. After putting an input it is outputting "e" multiple times.

Answer 1

You have a lot of errors

1. You allocate extra space for the input string copy, you should not multiply by sizeof(int) because you need l chars not l ints, and that's partially true, because you also need the '\0' terminating character, so you need 1 + l chars, hence the first malloc line should read

   tab2 = malloc(1 + l);
   

2. You check the success of malloc but still continue to dereference a potentially NULL pointer, it doesn't matter what you do in case of malloc failure but you cannot derefrence a NULL pointer, this

   if (tab2 == NULL)
      printf("blad");
   

   should be something like

   if (tab2 == NULL)
      return -1; /* or anything that will abort execution */
   

3. You never copy the data read via fgets() to the newly allocated buffer, you should copy the bytes to the tab2.

   strcpy(tab2, tab1);
   

4. You don't add a terminating null byte after copying the characters in the deleteLastChar() function. You must add

   tab1[rozmiar - 1] = '\0';
   

   at the end of the for loop in deleteLastChar().

5. You free s in the deleteLastChar() function, after return, that line will never be reached, so it doesn't make sense, also you should not free that, because it's an input parameter to deleteLastChar() you should do it in the caller function instead.

This is a copy of your code, fixed

char *deleteLastChar(char* s)
{
    int rozmiar,i;
    char *tab1;

    if (s == NULL)
        return NULL;
    rozmiar = strlen(s);
    if(rozmiar == 0)
        return NULL;
    tab1 = malloc(rozmiar);
    if (tab1 == NULL)
        return NULL;
    for (i = 0 ; i < (rozmiar - 1) ; i++)
    {
        tab1[i] = s[i];
    }
    tab1[rozmiar - 1] = '\0';

    return tab1;
}


int main()
{
    char tab1[ROZ], *tab2;
    int l, i, p, a;

    printf("podaj napis\n");

    fgets(tab1, 100, stdin);

    l    = strlen(tab1);
    tab2 = malloc(1 + l);
    if (tab2 == NULL)
        return -1;
    strcpy(tab2, tab1);

    for (i = 0 ; i < l ; i++)
    {
        char *result;

        result = deleteLastChar(tab2);
        if (tab2 != NULL)
        {
            p = strlen(tab2);
            for (a = 0 ; a < p ; a++)
            {
                printf("%c", tab2[a]);
            }
            free(tab2);
            printf("\n");

            tab2 = result;
        }
        else
        {
            free(tab2);
            return -1;
        }
    }
    free(tab2);

    return 0;
}

BTW, doesn't it look more readable with the use of more whitespace?

Note: Oh, and there is no need to cast malloc in c, and fgets() consumes the trailing '\n', so you might want to remove that.

Answer 2

Right away I can see 2 issues with your code

• You are passing tab2 to deleteLastChar. tab2 is just allocated, data is not assigned to tab2. I think your intension is to pass tab1

• There is no point in calling free() after return statement in deleteLastChar()