1

I'm trying a new assignment I've got: I need to copy the Environment variables from the shell into an array, then lowercase them and print them back. (I'm using Ubuntu 20.04.4 LTS, gcc for compiling, and writing my code in C language). I was advised to use malloc to create the array but decided not to. everything goes good up to the point I'm trying, for my own reasons, to see what I've created. then, when running it via valgrind - I'm getting this error I'm not quite sure why and how to fix. I'd appreciate the help.

as for the code:

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

char PrintString(char **strings, int counter)
{
    for(int j = 0; strings[j] != NULL; j++)
    {
        printf("%s \n", strings[j]);
    }
return (4);
}
/*----------------------------------------------*/
//function to lowercase the strings
char Lowercase(char **array, int seq_num) //seq_num is the size of the array
{
    int i;
    int j;
    for (i = 0; i < seq_num; i++) //for every line in the env list
    {
        for(j = 0; j < strlen(array[i]); j++) //for every character in the env strings
        {
            array[i][j] = tolower(array[i][j]);
        }
        printf("%s\n", (array[i]));
    }
    return(**array);    
}
/*----------------------------------------------*/
//in order to get the number of the env var - this is the block
int Conut_variables(char **envp)
{
        int counter=0; //counter for the no. of variables
        for(int i = 0; envp[i] != NULL ; i++)
        { 
            counter++;
        }
        return(counter);
}
/*----------------------------------------------*/
int main(int argc, char *argv[], char *envp[])
{
    int counter = Conut_variables(envp);
    int i,j;
    char *new_buffer[counter];
    for(i = 0; envp[i] != NULL; i++)
    {
        new_buffer[i] = envp[i];
    }
    Lowercase(new_buffer, counter);
    PrintString(new_buffer, counter);
return (0);
}       
/*----------------------------------------------*/

as for the errors:

==20052== Conditional jump or move depends on uninitialised value(s)
==20052==    at 0x10922E: PrintString (Ex4vol2.c:9)
==20052==    by 0x1094CB: main (Ex4vol2.c:53)
==20052==  Uninitialised value was created by a stack allocation
==20052==    at 0x109364: main (Ex4vol2.c:44)
==20052== 
==20052== 
==20052== HEAP SUMMARY:
==20052==     in use at exit: 0 bytes in 0 blocks
==20052==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==20052== 
==20052== All heap blocks were freed -- no leaks are possible
==20052== 
==20052== For lists of detected and suppressed errors, rerun with: -s
==20052== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
dbush
  • 205,898
  • 23
  • 218
  • 273
  • 1
    Right now there's really no difference between your `new_buffer` and the original `envp`, as both contains the exact same pointers. ***Except*** the terminating null pointer. – Some programmer dude May 20 '22 at 12:30
  • 1
    Also, why do `Lowercase` and `PrintString` return values? Why do you return the values that you do return? – Some programmer dude May 20 '22 at 12:31
  • [is that a valid signature for `main`?](https://stackoverflow.com/questions/2108192/what-are-the-valid-signatures-for-cs-main-function) – yano May 20 '22 at 12:32
  • 2
    @yano Afaik, it's a standard extension in Posix. – Ted Lyngmo May 20 '22 at 12:37
  • @Someprogrammerdude i understand what you're saying, but as the assignment was to copy those arguments into another array - that's what i did. what did you mean EXCEPT? – Gilad Tayeb May 20 '22 at 12:39
  • 1
    @TedLyngmo interesting.. never seen it. Thanks – yano May 20 '22 at 12:42
  • You do not copy the strings themselves, you only copy the *pointers*. The pointers in the `new_buffer` array all points to the exact same strings as `envp`. – Some programmer dude May 20 '22 at 13:52

1 Answers1

2

In PrintString you're not using the right loop control:

for(int j = 0; strings[j] != NULL; j++)

You pass int counter which tells you how many entries you have, but you don't use it. You instead look for a NULL entry, but you never set a NULL entry nor do you allocate space in the array for it.

So use counter instead.

for(int j = 0; j<counter; j++)
dbush
  • 205,898
  • 23
  • 218
  • 273
  • thank you for that. after fixing it - still same problem. i assume it wasn't related, but worth changing it anyway – Gilad Tayeb May 20 '22 at 12:37
  • @GiladTayeb Are you sure you didn't change something else? Valgrind ran clean for me after changing that one line. – dbush May 20 '22 at 12:42
  • i only changed what you told me. – Gilad Tayeb May 20 '22 at 12:44
  • @GiladTayeb I did the same change (but dbush was quicker to answer). It _is_ this problem. If you still get the same valgrind output, you haven't recompiled your program. – Ted Lyngmo May 20 '22 at 12:47
  • 1
    @TedLyngmo thank you both! i'm not sure what happened with my original file, but i've created a new file with the same code. then i edited the line dbush suggested - and it got fixed! thank you all – Gilad Tayeb May 20 '22 at 12:51