0

I have a structure:

struct s 
{
    unsigned size;
    char *var;
};

And an array of pointers to structures

for(i=0;i<rows;i++)
{
    for(j=0;j<columns;j++)
    {
        array[i][j] = (struct s*)malloc(sizeof(struct s));
        printf("Give the size to structure nr. %d: \n",i*columns+j+1);
        scanf("%d",&(array[i][j]->size));
    }
}   

and a function which returns a pointer to an array of random characters with a size chosen by user:

char* ChainOfCharacters(int liczba)
{
    int i;
    char *tabc = NULL;
    tabc = (char*)malloc(liczba*sizeof(char));
    if(tabc==NULL) exit(-1);

    else
    {
        char *tab=NULL;
        tab=tabc;
        for(tabc;tabc<tab+liczba;tabc++)
        {
            *tabc=rand()%(122-97)+97;
        }
    return tabc;    
    }
}

How to connect an array of characters which is a result of this function to pointer char *var of the structure s by using an array of pointers to structures? Is there any mistake in my code?

I tried this: array[i][j]->var=ChainOfCharacters(array[i][j]->size); but it seems to be wrong because i have wrong characters while trying to check this by printf. I don't even know how to write the array of characters on the screen by printf. I tried this printf("%c ", *(array[i][j]->var)); but it shows totaly random signs. I will be very thankful for answers for my questions :)

5
  • 1
    First: minimal reproducible example. What is this function lancuch() for example? Then, there's at least one bad error in your ChainOfCharacters, you're returning tabc, which points one past the last character you wrote to your memory. Even if you fix this to return tab instead, your result isn't a string (it doesn't have a NUL byte at the end), so you can't just print this with e.g. printf("%s" ...). Commented Aug 27, 2017 at 12:35
  • Thanks, I edited and changed lancuch() for ChainOfCharacters. I need just a chain of characters, not string so it's good. So it's the second problem to print character after character. I've already changed tabc for tab in return but it didn't helped ;/ Commented Aug 27, 2017 at 12:48
  • Please study the concept of a minimal reproducible example again. Then please make one. Commented Aug 27, 2017 at 12:53
  • 1
    further unrelated comments: 1. don't cast the result of malloc in C, 2. sizeof(char) is 1 by definition, no need to multiply by it, 3. Initializing a pointer to NULL only to immediately assign it a different value is useless, initialize it directly to the desired value, 4. use some spaces to make your code readable, 5. an else after exit() is useless, the code can't be reached ... Commented Aug 27, 2017 at 12:55
  • For finding the error, a debugger will be very helpful ... and for basic debugging strategies without that, read How to debug small programs Commented Aug 27, 2017 at 12:56

2 Answers 2

2

First, let's straighten out a few issues with the posted code. size_t is the correct type for array indices and sizes, so the .size member of the s structure should have type size_t. This also means that the ChainOfCharacters() function should take an argument of type size_t, and this has ramifications for the format string specifiers in the calls to printf() and scanf().

A user may not enter a number at the input prompt, in which case no assignment would be made; but since the posted code does not check the value returned from scanf() to verify that a correct input was made, the code would continue with an indeterminate value in that .size field, leading to undefined behavior. The code below checks for this, and exits with an error message if the user fails to input a number here, though this input validation could be further improved.

Note that it is better to use the EXIT_FAILURE macro than to use -1, as this is clearer and more portable.

The ChainOfCharacters() function does not return a string, but only a pointer to an array of characters, so the characters will need to be printed one-by-one.

Note that there is no need to cast the result of malloc() in C, and it is better to use identifers rather than explicit types for operands of the sizeof operator: this is less error-prone and easier to maintain when types change.

The loop that assigns characters in the ChainOfCharacters() function is needlessly complex and contains an error, possibly as a result of this complexity; the tabc pointer points to one past the end of the allocated storage when it is returned. This can be resolved by rewriting the loop to use an index instead of pointer arithmetic. It is generally clearer and less error-prone to use array indexing instead of pointer arithmetic when possible. Avoid using magic numbers: rand() % ('z' - 'a') + 'a' is much clearer in intent than rand()%(122-97)+97 (and don't be afraid to use a little whitespace). Further, note that the C Standard makes few restrictions on the character encodings that may be used by an implementation, and in particular this is not required to be ASCII. The letters of the Latin alphabet need not even be encoded in a contiguous sequence, as is the case with EBCDIC (which still exists in the real world). This is unlikely to be a problem here, but know that there are portable ways to handle this issue.

To assign the result from ChainOfCharacters(), simply assign the pointer to the appropriate array[][] field:

array[i][j]->var = ChainOfCharacters(array[i][j]->size);

To print the contents of the .var fields, iterate over the array, and for each struct, loop over the contents of the allocated storage for .var, printing the characters:

/* print characters */
for (size_t i = 0; i < rows; i++)
{
    for (size_t j = 0; j < columns; j++)
    {
        printf("array[%zu][%zu]->val: ", i, j);
        for (size_t k = 0; k < array[i][j]->size; k++) {
            putchar(array[i][j]->var[k]);
        }
        putchar('\n');
    }
}

After all of this, you will need remember to free() the allocated memory. Here is a complete program that implements these changes.

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

struct s 
{
    size_t size;
    char *var;
};

char* ChainOfCharacters(size_t liczba);

int main(void)
{
    size_t rows = 3;
    size_t columns = 3;
    struct s *array[rows][columns];

    for (size_t i = 0; i < rows; i++)
    {
        for (size_t j = 0; j < columns; j++)
        {
            array[i][j] = malloc(sizeof *array[i][j]);
            printf("Give the size to structure nr. %zu: \n",
                   i * columns + j + 1);

            if (scanf("%zu", &(array[i][j]->size)) != 1) {
                fprintf(stderr, "Incorrect input\n");
                exit(EXIT_FAILURE);
            };
            array[i][j]->var = ChainOfCharacters(array[i][j]->size);
        }
    }

    /* print characters */
    for (size_t i = 0; i < rows; i++)
    {
        for (size_t j = 0; j < columns; j++)
        {
            printf("array[%zu][%zu]->val: ", i, j);
            for (size_t k = 0; k < array[i][j]->size; k++) {
                putchar(array[i][j]->var[k]);
            }
            putchar('\n');
        }
    }

    /* free allocated memory */
    for (size_t i = 0; i < rows; i++)
    {
        for (size_t j = 0; j < columns; j++)
        {
            free(array[i][j]->var);
            free(array[i][j]);
        }
    }

    return 0;
}

char* ChainOfCharacters(size_t liczba)
{
    char *tabc = NULL;
    tabc = malloc(sizeof *tabc * liczba);
    if (tabc == NULL) {
        exit(EXIT_FAILURE);
    } else {
        for (size_t i = 0; i < liczba; i++) {
            tabc[i] = rand() % ('z' - 'a') +'a';
        }
        return tabc;
    }
}

Sample interaction:

Give the size to structure nr. 1: 
1  
Give the size to structure nr. 2: 
2
Give the size to structure nr. 3: 
3
Give the size to structure nr. 4: 
9
Give the size to structure nr. 5: 
8
Give the size to structure nr. 6: 
7
Give the size to structure nr. 7: 
4
Give the size to structure nr. 8: 
5
Give the size to structure nr. 9: 
6
array[0][0]->val: i
array[0][1]->val: lc
array[0][2]->val: psk
array[1][0]->val: lryvmcpjn
array[1][1]->val: bpbwllsr
array[1][2]->val: ehfmxrk
array[2][0]->val: ecwi
array[2][1]->val: trsgl
array[2][2]->val: rexvtj

On choosing correct types

As I said at the beginning of the answer, size_t is the correct type for array indices, as it is an unsigned type that is guaranteed to be able to hold any array index value. But, unsigned is also fine, though unsigned int and size_t may not have the same ranges.

A significant problem in the OP code is that the .size field is unsigned, while the scanf() statement that stores input in this field uses the d conversion specifier, which is meant to be used with ints. According to the Standard, mismatched conversion specifiers and arguments lead to undefined behavior, which includes appearing to work in some instances. But you can't rely on undefined behavior doing what you expect. In the posted code,%u should have been used to store an unsigned value in the .size field. Further, the ChainOfCharacters() function was declared to accept an argument of type int, but was called with an unsigned argument (from .size). This may also lead to implementation-defined behavior, since an unsigned value may not be representable in an int.

Another place that this problem could arise is in the loop that prints the characters. For example, consider:

struct s 
{
    unsigned size;
    char *var;
};

/* ... */

int rows = 3;
int columns = 3;
struct s *array[rows][columns];

/* ... */

/* print characters */
for (int i = 0; i < rows; i++)
{
    for (int j = 0; j < columns; j++)
    {
        printf("array[%d][%d]->val: ", i, j);
        for (int k = 0; k < array[i][j]->size; k++) {
            putchar(array[i][j]->var[k]);
        }
        putchar('\n');
    }
}

Here, k is a signed int, while array[i][j]->size is an unsigned int type, so the value of k will be converted to an unsigned value before the comparison is made. This conversion is well-defined, but can lead to surprises if k is negative.

Enabling compiler warnings will help to detect issues like this at compile time. I always use at least gcc -Wall -Wextra (and -Wpedantic too, but you can probably do without this). Enable compiler warnings, and pay attention to them, and fix them.

Sign up to request clarification or add additional context in comments.

10 Comments

@FelixPalmen-- all good and fair points. It was silly of me to use printf() instead of putchar(); my weak defense is that I had converted an earlier printf() containing other information, and didn't think. Concerning ASCII, I simply avoided the issue, but this is a valid point. I have updated my answer in both of these respects.
@FelixPalmen-- As for unsigned vs size_t, I agree that unsigned is not wrong, but I think that it is bad to default to this for array indices. OP seemed to be causal about using unsigned for the .size field, while taking input with the %d directive, and used the int argument liczba in the ChainOfCharacters() function, so I changed all to size_t for correctness and consistency (though correctness may slightly too strong of a term) without too much comment.
Well, thank you so much for help! The reason of my problem was that my pointer pointed one past the last character in array. So changing returned pointer helped and I didn't know how to print that array after connecting to structure, but this piece of code printf("%c ", array[i][j]->var[k]); helped enough :)
@PiotrWitkoś-- I am glad my answer helped. A simple change could make your original code "work", but there were some important issues that I brought up in my answer, and I hope that you understood them. It is very important in robust code to check the value returned from scanf() to avoid undefined behavior due to using a variable that has not been assigned a value yet. Also, unsigned and signed integer types were used too casually in the OP code, and this can lead to serious problems; in fact the posted code had undefined behavior. I have added to my answer to discuss this a bit.
@PiotrWitkoś-- What compiler are you using? malloc() returns a (void *) value, which is being assigned to tabc, which is a (char *). C allows you to assign a void pointer to a pointer of another object type, implicitly making the conversion. But, C++ does not allow this, and requires the cast. I suspect that you are using a C++ compiler.
|
0

In your ChainOfCharacters() function, you should be returning tab, not tabc. tab is pointing to the beginning of your chain, tabc is pointing to the end of it.

3 Comments

It would be helpful if whoever downvoted my answer could tell me why. I made the OP's code work by simply making the small change I said, so I really don't see why it was downvoted.
I didn't cast the DV, but while this answer may solve the immediate problem, the posted code has other issues. Significantly, if compiled with warnings enabled, there will be warnings about an incorrect format specifier (%d expects an int, not an unsigned int), and perhaps a comparison between signed and unsigned types in the loop to print characters, e.g., with for (int k = 0; k < array[i][j]->size; k++) {}. I think that the answer is somewhat incomplete, but not deserving of a DV; yet everyone has their own opinion of which answers deserve UVs and which deserve DVs....
savram I think that someone downvoted your answer, because you wrote something that was written on this topic two times in earlier discusions. But im a am thankful for your answer so I upvoted that :D

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.