0

I'm trying to create a program which the user inputs the number of items (rows) and give each one of them a name (scanf) with the max of 30 characters.

I want to create this code with pointers of pointers, once that I'm learning this on C.

I'm having some difficulties with the code.

Draft of the 2D array.

Code snippet:

PS: #define MAX 31


  char **items = NULL, *columns = NULL, name[MAX];
  int rows, aux_rows;

  printf("\nNumber of items:\n");
  scanf("%d", &rows);

  items = (char **) malloc(rows * sizeof(char*));

  for (aux_rows = 0; aux_rows < rows; aux_rows++){
    columns[aux_rows] = (char *) malloc(MAX * sizeof(char));
  }

  for (aux_rows = 0; aux_rows < rows; aux_rows++){
    printf("\nName of item %d:\n", (aux_rows + 1));
    scanf("%s", name);
    *items[aux_rows] = name;
  }
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
Samuel V.
  • 7
  • 2
  • 2
    You can't copy arrays with a simple assignment statement. Use the `strcpy` function. Also, change `scanf("%s", name);` to `scanf("%30s", name)` to prevent evil users from overflowing the `name` array. – user3386109 May 12 '22 at 22:28
  • The expression `*items[aux_rows]` will dereference the pointer which is pointing to the first character, so it will yield a single character. This is not what you want. As already stated, you want to use `strcpy` instead. – Andreas Wenzel May 12 '22 at 22:33
  • @user3386109: In this case, it would probably be better to use `fgets` instead of `scanf`, especially if the "Name of item" can consist of more than one word (e.g. `"candy bar"`). The function `scanf` with the `%s` conversion format specifier will only read a single word. – Andreas Wenzel May 12 '22 at 22:34
  • https://asciiflow.com/ is nice to generate simple graphics. Better than using png files. – Franck May 12 '22 at 22:59
  • @user3386109 Thanks for your suggestion. I didn't know this trick. I'll definitely use it! – Samuel V. May 13 '22 at 21:12
  • @AndreasWenzel Thanks, I didn't know the `strcpy` function as well. I plan using `scanf` anyway, the items should have only one word. – Samuel V. May 13 '22 at 21:17
  • @Franck That's a great tool! I'm bookmarking it, thanks. – Samuel V. May 13 '22 at 21:19

2 Answers2

1

items was allocated and not columns. And use strcpy to copy the characters.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX 31

int main()
{
    char **items = NULL, *columns = NULL, name[MAX];
    int rows, aux_rows;

    printf("\nNumber of items:\n");
    scanf("%d", &rows);

    items = (char **)malloc(rows * sizeof(char *));

    for (aux_rows = 0; aux_rows < rows; aux_rows++)
    {
        items[aux_rows] = malloc(MAX * sizeof(char));
    }

    for (aux_rows = 0; aux_rows < rows; aux_rows++)
    {
        printf("\nName of item %d:\n", (aux_rows + 1));
        scanf("%s", name);
        strcpy(items[aux_rows], name);
    }
    return 0;
}
$ gcc array2d.c
$ ./a.out      

Number of items:
2

Name of item 1:
Hello 

Name of item 2:
World!
$ 
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
Franck
  • 1,340
  • 2
  • 3
  • 15
1
*items[aux_rows] = name;

is wrong on two counts.

Both * and [] dereference their unary operand. If items is a char **, items[n] is a char *, and *items[n] is a char.

This attempts to assign an array to the first element of each buffer.

Secondly, arrays cannot be copied by assignment. Use strcpy to copy strings from one buffer to another.

That said, you could simply read your strings directly into the pre-allocated buffers, and do away with the temporary buffer.


In this line,

columns[aux_rows] = (char *) malloc(MAX * sizeof(char));

columns should be items.


Some things of note:

sizeof (char) is guaranteed to be 1. Its use is superfluous.

The return of malloc should not be cast in C.

malloc can fail. scanf can fail. You should get in the habit of not ignoring return values.

scanf("%s", ...) is as dangerous as gets. At a minimum, use field-width specifiers to limit input (should be the size of your buffer minus one).

char foo[128];
if (1 != scanf("%127s", foo))
    /* handle error */;

Note that using the %s limits input to not contain any whitespace. scanf in general is a terrible tool, consider a line based approach using fgets.

With that said, the minimum changes to make this reasonably safe:

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

#define MAX 31

void die(const char *msg)
{
    fprintf(stderr, "%s\n", msg);
    exit(EXIT_FAILURE);
}

int main(void)
{
    size_t rows;

    printf("Number of items: ");

    if (1 != scanf("%zu", &rows))
        die("Failed to read input.");

    char **items = malloc(sizeof *items * rows);

    if (!items)
        die("Failed to allocate memory.");


    for (size_t i = 0; i < rows; i++) {
        if (!(items[i] = malloc(MAX)))
            die("Failed to allocate row.");

        printf("Name of item %zu: ", i + 1);

        if (1 != scanf("%30s", items[i]))
            die("Failed to read item input.");
    }

    for (size_t i = 0; i < rows; i++) {
        puts(items[i]);
        free(items[i]);
    }
}
Oka
  • 23,367
  • 6
  • 42
  • 53