Memory Allocation, Outputting Garbage

Question

Why is this outputting garbage instead of char values in arrayLetter? When I put the for loop to print the arrayLetter above the method calls it works; when I put it below it, it doesn't work. I did not change arrayLetter in the methods or in the main. This is a simple program that shifts index and output the shifted alphabets.

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

int* getMatchedIndex(char arrayLetter[], char plainTextExample[],int sizeArrayLetter, int sizePlainTextExample);
int* vigenereCipherEncryptIndex(int key[], int sizeKey, int sizeArrayLetter, int sizePlainTextExample, int arrayMatchAtIndex[]);
char* convertToNewEncryptedArray(char arrayLetter[], int encryptArray[], int sizeArrayLetter, int sizePlainTextExample);

int main(void) {

    char arrayLetter[] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
                           'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r',
                           's', 't', 'u', 'v', 'w', 'x', 'y', 'z'};

    int key[] = { 21, 4, 2, 19, 14, 17};

    char plainTextExample[] = { 'h', 'e', 'r', 'e',
                                'i', 's',
                                'h', 'o', 'w',
                                'i', 't',
                                'w', 'o', 'r', 'k', 's'};

    int sizeArrayLetter = sizeof(arrayLetter)/sizeof(arrayLetter[0]);
    int sizePlainTextExample = sizeof(plainTextExample)/sizeof(plainTextExample[0]);
    int sizeKey = sizeof(key)/sizeof(key[0]);

    int *arrayMatchAtIndex;
    int *encryptArray;
    char *cipherArray;

    arrayMatchAtIndex = getMatchedIndex(arrayLetter, plainTextExample, sizeArrayLetter, sizePlainTextExample);
    encryptArray = vigenereCipherEncryptIndex(key, sizeKey, sizeArrayLetter, sizePlainTextExample, arrayMatchAtIndex);
    cipherArray = convertToNewEncryptedArray(arrayLetter, encryptArray, sizeArrayLetter, sizePlainTextExample);

    for(char i=0; i < sizeArrayLetter; i++)
    {
        //printf("%s%i%s", "Indexes of Encryption: ", encryptArray[i], " \n");
        printf("%s", arrayLetter);

    }


    free(arrayMatchAtIndex);
    free(encryptArray);
    free(cipherArray);

    return EXIT_SUCCESS;
}

//compare arrays and store index into array
int *getMatchedIndex(char arrayLetter[], char plainTextExample[],int sizeArrayLetter, int sizePlainTextExample)
{

    int* arrayMatchAtIndex = malloc(sizePlainTextExample* sizeof(int));

    //loop plainTextExample
    for(int i=0; i < sizePlainTextExample ; i++)
    {
        //loop arrayLetter
        for(int j=0; j < sizeArrayLetter ; j++)
        {
            //compare -> match then store arrayLetter index into arrayMatchAtIndex
            if(plainTextExample[i] == arrayLetter[j])
            {
                arrayMatchAtIndex[i] = j;
            }
        }

    }
    return arrayMatchAtIndex;
}

//encrypt array by adding 'key' indexes into array
int* vigenereCipherEncryptIndex(int key[], int sizeKey, int sizeArrayLetter, int sizePlainTextExample, int arrayMatchAtIndex[])
{
    //make new array
    int* encryptArray = malloc(sizePlainTextExample* sizeof(int));

    int j=0;

    //encrypt element -> (add elements of arrayIndexes and key) mod 26
    for(int i=0; i < sizePlainTextExample; i++)
    {

        if(i >= sizeKey)
        {
            j = (i % sizeKey);
            key[i] = key[j];
        }

        encryptArray[i] = (arrayMatchAtIndex[i]+key[i]) % 26;
    //  printf("%s%d%s%d%s", "Match Element: ", arrayMatchAtIndex[i], " key ", key[i], " ");
    //  printf("%s%d%s", "encryptElement: ", encryptArray[i], " \n");
    }

    return encryptArray;
}

//convert encrypt array index to its elements
char* convertToNewEncryptedArray(char arrayLetter[], int encryptArray[], int sizeArrayLetter, int sizePlainTextExample)
{
    char* cipherArray = malloc(sizePlainTextExample* sizeof(int));

    //encrypted index - > convert elements to new index
    for(int i=0; i < sizePlainTextExample ; i++)
    {
        for(int j=0; j< sizeArrayLetter; j++)
        {
            //find index->match then put Encrypted index into letter element
            if(encryptArray[i] == j)
            {
                cipherArray[i] = arrayLetter[j];
                printf("%c", arrayLetter[2]);
            }

        }
    }

    return cipherArray;
}

Since `arrayLetter` is not null-terminated, you will get garbage when you print it with `printf("%s", arrayLetter);` — there's nothing that lets `printf()` stop processing when it reaches the end of the array. — Jonathan Leffler, Mar 14 '18 at 07:01
So, what would be the correct way to fix this? What do you mean null-terminated? — NeedHlub, Mar 14 '18 at 07:08
In C, a string is a sequence of characters up to the first null byte — the null byte marks the end of the string, or terminates it. Your `arrayLetter` doesn't have a null byte in it, so it isn't a string. The `%s` format is intended for printing strings; printing non-strings leads to undefined behaviour, and frequently results in garbage being printed. You could fix it by using `printf("%*s", (int)sizeof(arrayLetter), arrayLetter);` — the `*` means take an `int` value as the maximum number of characters to print, stopping if it encounters a null byte earlier. — Jonathan Leffler, Mar 14 '18 at 07:12
The solution code suggested didn't seem to fix the error. The "%*s" is good to know though. — NeedHlub, Mar 14 '18 at 07:22
apart from other reason statement `key[i] = key[j];` causing garbage data. what it seems to do ? there are only 6 elements in `key` and the block `if(i >= sizeKey)` is accessing out of range elements bcz loop rotates `sizePlainTextExample` times. — Achal, Mar 14 '18 at 07:27
OK; then I've not looked at enough of your code, and won't be doing so for a few hours as it is bedtime where I'm living. — Jonathan Leffler, Mar 14 '18 at 07:27
@achal I want the key to "loop" -> {21, 4, 2, 19, 14, 17} {21, 4, 2, 19, 14, 17} etc. so the index will keep adding...jonathanLeffler: thanks for the looking at my code :) — NeedHlub, Mar 14 '18 at 07:32
@JonathanLeffler `%*s` is not a solution here, it just let specify the preferred size of the field, if the string is shorter padding will appear, but if the string is longer than the field, the printing will just silently overflow. — Jean-Baptiste Yunès, Mar 14 '18 at 09:01
@Jean-BaptisteYunès: You're right; I mistyped, omitting the crucial `.` — I intended `printf("%.*s", sizeArrayLetter, arrayLetter);` (or the equivalent with `sizeof()` and a cast). It may or may not be relevant, but `arrayLetters` gets over-written by `vigenereCipherEncryptIndex()` and the result contains a lot of control characters (mostly ^B, `'\002'`). — Jonathan Leffler, Mar 14 '18 at 14:40
I've taken another look at the code, and I confess that I'm not sure what you're algorithm is. It is different from any of the Vigenere encryption algorithms I've seen before — and I've seen a fair few. I find the code hard to read, too. The names are long and sort of self-explanatory, but they're unreadable too. To the extent I understand what you're doing, you first scan the text to be encrypted and create an array that identifies somehow which characters are going to be encoded (that's `getMatchedIndex()`). _[…continued…]_ — Jonathan Leffler, Mar 18 '18 at 02:23
_[…continuation…]_ You then call `vigenereCipherEncryptIndex()`, which allocates an array for the message and converts each character in the plaintext into an offset through the alphabet (so `'a'` is treated as 0), but it includes the key too. Then finally you call `convertToNewEncryptedArray()` to post-process the result from the previous function, converting the numbers from 0..25 back to letters `'a'` to `'z'`, I think? The final printout is then based on the size of the alphabet and not on the size of the message, which most peculiar. _[…continued…]_ — Jonathan Leffler, Mar 18 '18 at 02:29
_[…continuation…]_ Normally, people manage to do the encoding in a single function that simply steps through the plain text and the key, producing one output letter at a time. If there's a need to preserve the input text, then you need to allocate as much space as the plain text uses for the cipher text (including a null byte to terminate the string). I trust you've rethought your whole approach — and got it all working. There are many questions about Vigenere cipher, often associated with the [tag:cs50] tag. They'll show you how other people have worked it out. — Jonathan Leffler, Mar 18 '18 at 02:33

score 0 · Answer 1 · answered Mar 14 '18 at 08:59

0

As Jonathan Leffler says clearly, your array is just an array, not a C-string (sequence of chars nul-terminated). Use %s only with C-strings.

Change your loop, to:

for (char i=0; i<sizeArrayLetter; i++) {
    putchar(arrayletter[i]);
}
putchar('\n');

This will print individually all chars from the array and an additional end-of-line.

answered Mar 14 '18 at 08:59

Jean-Baptiste Yunès

34,548
4
48
69

It still prints out garbage. Is that code the same as this: ~ for(char i=0; i < sizeArrayLetter; i++) { printf("%c", arrayLetter[i]); } ~ – NeedHlub Mar 14 '18 at 12:41
That means that you overwrite that array accidentally. – Jean-Baptiste Yunès Mar 14 '18 at 12:47
Yeah, it seems like it. I don't know where though. – NeedHlub Mar 14 '18 at 12:55

Memory Allocation, Outputting Garbage

1 Answers1