0

I am currently writing a program that creates a key-value store in shared memory. The code of interest is in the kv_store_write() function but I included the rest for context.

The memory will ultimately be shared between various processes who will read and write values to the same memory space. a hash function is used to increase searching speeds and determine location of storage.

My current challenge is in storing the key-value pair in a structured way and accessing that structure. I opted for nested structs as defined bellow. In using mmap() I cast the void pointer to the address returned to a pointer to a struct I defined: Pod. in kv_store_write() I attempt to access the value and key

The compilation error I get are not helping me make sense of what is going wrong. Am I accessing the elements of my structure appropriately?

I am confused as to when to use -> and .

Edits: I have added the read function for completeness. Here are the compilation errors I get:

Assignement_2_1.c: In function ‘kv_store_read’:
Assignement_2_1.c:142:17: error: assignment to expression with array type
    valueInStore = memEthienne -> pod[hashedPod].value[podIndex];
                 ^
Assignement_2_1.c:143:11: warning: passing argument 1 of ‘strcpy’ from incompatible pointer type [-Wincompatible-pointer-types]
    strcpy(value, valueInStore);// might be a problem with the way I am copying the value
           ^
In file included from Assignement_2_1.c:6:0:
/usr/include/string.h:125:14: note: expected ‘char * restrict’ but argument is of type ‘char **’
 extern char *strcpy (char *__restrict __dest, const char *__restrict __src)
              ^
Assignement_2_1.c:143:18: warning: passing argument 2 of ‘strcpy’ from incompatible pointer type [-Wincompatible-pointer-types]
    strcpy(value, valueInStore);// might be a problem with the way I am copying the value
                  ^
In file included from Assignement_2_1.c:6:0:
/usr/include/string.h:125:14: note: expected ‘const char * restrict’ but argument is of type ‘char **’
 extern char *strcpy (char *__restrict __dest, const char *__restrict __src)
              ^
Assignement_2_1.c:144:11: warning: return from incompatible pointer type [-Wincompatible-pointer-types]
    return (value);
           ^
Assignement_2_1.c:144:11: warning: function returns address of local variable [-Wreturn-local-addr]
Assignement_2_1.c:149:1: error: expected declaration or statement at end of input
 }

Here is the code

#include <fcntl.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>

//----------------------------------------------------------------------

#define SIZE_OF_KEY 32
#define SIZE_OF_VALUE 256
#define NUMBER_OF_KEYVALUE_PAIRS 256
#define NUMBER_OF_PODS 256
#define SIZE_OF_INDEX 1
#define NUMBER_OF_INDEX 1
#define MEMORY_LENGTH (NUMBER_OF_PODS*NUMBER_OF_KEYVALUE_PAIRS*(SIZE_OF_VALUE + SIZE_OF_KEY + SIZE_OF_INDEX))

//-------------------------------------------------------------------
typedef char KEY[SIZE_OF_KEY];
typedef char VALUE[SIZE_OF_VALUE];

typedef struct{
    KEY key[NUMBER_OF_KEYVALUE_PAIRS];
    VALUE value[NUMBER_OF_KEYVALUE_PAIRS];
    int index; 
}keyValue;

typedef struct{
    keyValue pod[NUMBER_OF_PODS];
}Pod;

//typedef keyValue Pod[NUMBER_OF_PODS];


int kv_store_create(char *name);
int kv_delete_db();
int kv_store_write(char *key, char *value);
char *kv_store_read(char *key);
char **kv_store_read_all(char *key);

char *DATABASE_NAME = "memoryEthienne";

//-------------------------------------------------------------------
int main(){
    kv_store_create(DATABASE_NAME);
}
//-------------------------------------------------------------------
int hash_func(char *word){
    int hashAddress = 5381;
    for (int counter = 0; word[counter]!='\0'; counter++){
        hashAddress = ((hashAddress << 5) + hashAddress) + word[counter];
    }
    return hashAddress % NUMBER_OF_PODS < 0 ? -hashAddress % NUMBER_OF_PODS : hashAddress % NUMBER_OF_PODS;
}
//-------------------------------------------------------------------
int kv_store_create(char *name){
    int fd = shm_open(name, O_CREAT|O_RDWR, S_IRWXU); // shm_open return a file descriptor
    Pod* memEthienne = mmap(NULL, MEMORY_LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); // Makes the memory element sharable and useable in the process and returns a pointer to the adress of the memory for the particular process memory space

    ftruncate(fd, MEMORY_LENGTH); // This allocates the proper size to memory
    close(fd);
    DATABASE_NAME = name; // Could be a good idea to initialise the indexes in the creation function
}
//-------------------------------------------------------------------
int kv_store_write(char *key, char *value){

    if (strlen(key) > SIZE_OF_KEY){
        fprintf(stderr, "Please enter a valid length for the key\n" );
    }
    if (strlen(value) > SIZE_OF_VALUE){
        fprintf(stderr, "Please enter a valid length for the value\n" );
    }

    struct stat s;

    int fd = shm_open(DATABASE_NAME, O_RDWR, 0);
    if (fd < 0)
        printf("Error...opening shm\n");

    if (fstat(fd,&s) == -1){
        printf("Error fstat\n");
    }

    Pod* memEthienne = mmap(NULL, MEMORY_LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); // This returns an adress to shared memory in the and allocates it as a pointer to an array of structs
    close(fd);

    int hashedPod = hash_func(key);
    int podIndex = memEthienne -> pod[hashedPod].index;

       memset(&(memEthienne -> pod[hashedPod].key[podIndex]), 0, SIZE_OF_KEY);
       memset(&(memEthienne -> pod[hashedPod].value[podIndex]), 0, SIZE_OF_VALUE); //void *memset(void *s, int c, size_t n);
       strncpy(memEthienne -> pod[hashedPod].key[podIndex], key, strlen(key)); // strncpy((har *dest, const char *src, size_t n)
       strncpy(memEthienne -> pod[hashedPod].value[podIndex], value, strlen(value));
       if (podIndex < 255){
            (memEthienne -> pod[hashedPod].index)++;
        }
        else
        {
            memEthienne -> pod[hashedPod].index = 0;    
        }
}

//-----------------------------------------------------------------
char *kv_store_read(char *key){ // This function returns a copy of the string currently stored at the location in memory associated with the key
// 1 - Check value entered makes sense
// 2 - Hash the key
// 3 - Create a search counter
// 4 - Search the pod with the hashed key until the (current index -1) and return the associated value
// QUESTION : Am I opening the memory with the appropriate permissions
if (strlen(key) > SIZE_OF_KEY){
        fprintf(stderr, "Please enter a valid length for the key\n" );
struct stat s;
int fd = shm_open(DATABASE_NAME, O_RDWR, 0);
    if (fd < 0)
        printf("Error...opening shm\n");

    if (fstat(fd,&s) == -1)
        printf("Error fstat\n");

    Pod* memEthienne = mmap(NULL, MEMORY_LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); // This returns an adress to shared memory in the and allocagted it as a pointer to an array of structs
    close(fd);

    int hashedPod = hash_func(key);
    int podIndex = memEthienne -> pod[hashedPod].index;
    char* value[SIZE_OF_VALUE];
    char* valueInStore[SIZE_OF_VALUE];
    int searchIndex = 0;
    while (searchIndex < podIndex){

        if (strcmp((memEthienne -> pod[hashedPod].key[searchIndex]),key) == 0){
            valueInStore = memEthienne -> pod[hashedPod].value[podIndex];
            strcpy(value, valueInStore);// might be a problem with the way I am copying the value
            return (value);
        }
        searchIndex ++;
    }
    fprintf(stderr, "Unable to locate the requested key\n" );
}
//------------------------------------------------------------------
Ethienne
  • 111
  • 1
  • 3
  • 12
  • When asking questions about build errors, always *include* the errors. Please edit your question to include the errors, in full, complete and with any possible informational notes. Also please point out (with e.g. comments) where in the shown code the errors are. – Some programmer dude Mar 12 '17 at 22:47
  • If you use GCC, please use `-Wall` flag (to include all compile time warnings), and the entire output from GCC when compiling your program. – Nominal Animal Mar 12 '17 at 22:48
  • 1
    A couple of things though: Remember that arrays naturally decays to pointers to their first element, so the address-of operator in e.g. `&(memEthienne -> pod[hashedPod].key[podIndex])` is not needed. You also forget that strings in C are *terminated* and that terminator needs space as well, which means your string length check in `kv_store_write` have an off-by-one error. – Some programmer dude Mar 12 '17 at 22:51
  • 1
    Oh and one very important thing: The size of a structure can be *larger* than the size of each element in the structure. Please read [Why isn't sizeof for a struct equal to the sum of sizeof of each member?](http://stackoverflow.com/questions/119123/why-isnt-sizeof-for-a-struct-equal-to-the-sum-of-sizeof-of-each-member) for more information. That means your `MEMORY_LENGTH` calculation may be wrong. Wait, it's *definitely* wrong since `sizeof(int)` is *not* `1`. Use the `sizeof` operator to get the *actual* size (`sizeof(Pod)` give the *complete* size, including nested structures and arrays). – Some programmer dude Mar 12 '17 at 22:54
  • For a start, a cast is most times not a good idea, less for pointers. – too honest for this site Mar 12 '17 at 23:05
  • @Olaf, I open to implementing things differently, how would you go about accessing the memory block in a structures manner? This is out of the scope of the question in the post though, I'm not super familiar with the rules of forum – Ethienne Mar 12 '17 at 23:15
  • I yet have to find the cast for a recommendation, too bad you did not provide a [mcve] as required by [ask]. A cast is always the last ressort and most times an indicator of bad interface design. And it always has to be understood completely **why** one needs it, iff at all. – too honest for this site Mar 12 '17 at 23:20
  • All of the warning and error messages are pretty straight-forward and should be easy to understand. For example the first one: "assignment to expression with array type". You can not assign to an array, only copy to it. You can on the other hand assign to a *pointer*. Lets take another message: "function returns address of local variable". The variable `value` is a *local* variable, whose scope ends with the function, once the function returns the variable ceases to exist. Returning a pointer to it is bad. – Some programmer dude Mar 12 '17 at 23:33

1 Answers1

0

This line 

        valueInStore = memEthienne -> pod[hashedPod].value[podIndex];

errors because valueInStore is an array, not a pointer. vlueInsStore[0] would work because it is a pointer.

This line

   strcpy(value, valueInStore);

Errors because value is an array of char* pointers i.e. it has type char** in the context of a function argument - we say it "decays" to a pointer.

Also you attempt to return value even though it is an array declared on the stack which means its storage goes away as soon as you return.

JeremyP
  • 84,577
  • 15
  • 123
  • 161