I have a function which returns names of currently logged in users in a system. I am storing the names in char** array 'usernames' which is terminated with a NULL, so I can print the usernames using the while(usernames[i]!=NULL) loop. I have allocated 'users_amount + 1'(+1 for the NULL) memory for the pointers, but when I am using 'valgrind --tool=memcheck --leak-check=yes ./a.out' it displays these errors.
==3004== Memcheck, a memory error detector
==3004== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3004== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==3004== Command: ./a.out
==3004==
Amount of logged users : 1
User:user
==3004== Invalid read of size 8
==3004== at 0x10892E: main (test.c:57)
==3004== Address 0x522d648 is 0 bytes after a block of size 8 alloc'd
==3004== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3004== by 0x108823: get_usernames (test.c:30)
==3004== by 0x1088DD: main (test.c:54)
==3004==
==3004== Invalid read of size 8
==3004== at 0x108976: main (test.c:64)
==3004== Address 0x522d648 is 0 bytes after a block of size 8 alloc'd
==3004== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3004== by 0x108823: get_usernames (test.c:30)
==3004== by 0x1088DD: main (test.c:54)
==3004==
==3004==
==3004== HEAP SUMMARY:
==3004== in use at exit: 0 bytes in 0 blocks
==3004== total heap usage: 4 allocs, 4 frees, 1,421 bytes allocated
==3004==
==3004== All heap blocks were freed -- no leaks are possible
==3004==
==3004== For counts of detected and suppressed errors, rerun with: -v
==3004== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
Allocating 'users_amount + 2' memory leaves me with these errors ( they are referencing to the while loops in main):
==3016== Memcheck, a memory error detector
==3016== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3016== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==3016== Command: ./a.out
==3016==
Amount of logged users : 1
User:user
==3016== Conditional jump or move depends on uninitialised value(s)
==3016== at 0x108934: main (test.c:57)
==3016==
==3016== Conditional jump or move depends on uninitialised value(s)
==3016== at 0x10897C: main (test.c:64)
==3016==
==3016==
==3016== HEAP SUMMARY:
==3016== in use at exit: 0 bytes in 0 blocks
==3016== total heap usage: 4 allocs, 4 frees, 1,429 bytes allocated
==3016==
==3016== All heap blocks were freed -- no leaks are possible
==3016==
==3016== For counts of detected and suppressed errors, rerun with: -v
==3016== Use --track-origins=yes to see where uninitialised values come from
==3016== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
Setting usernames[users_amount + 1] to NULL seems to solve the problem as no errors appear. Why is allocating 'users_amount + 1' memory and setting usernames[users_amount] to NULL resulting in 'invalid read of size 8' errors ? Here is the code at the point where it does not show any errors:
#include <stdio.h>
#include <stdlib.h>
#include <pwd.h>
#include <sys/types.h>
#include <utmpx.h>
#include <string.h>
#include <unistd.h>
#include <grp.h>
int get_users_amount()
{
int counter = 0;
struct utmpx * user = getutxent();
while(user!=NULL)
{
if(user->ut_type == USER_PROCESS)
{
counter = counter + 1;
}
user = getutxent();
}
return counter;
}
char ** get_usernames()
{
int users_amount = get_users_amount();
char ** usernames;
usernames = malloc((users_amount + 2) * sizeof(char*));
usernames[users_amount] = NULL;
usernames[users_amount+1] = NULL;
setutxent();
struct utmpx * user = getutxent();
int counter = 0;
while(user!=NULL)
{
if(user->ut_type == USER_PROCESS)
{
usernames[counter] = strdup(user->ut_user);
counter = counter + 1;
}
user = getutxent();
}
return usernames;
}
int main()
{
printf("Amount of logged users : %d\n", get_users_amount());
char ** usernames = get_usernames();
int counter = 0;
while (usernames[counter]!=NULL)
{
printf("User:%s\n",usernames[counter]);
counter++;
}
counter = 0;
while (usernames[counter]!=NULL)
{
free(usernames[counter]);
counter++;
}
free(usernames);
return 0;
}