segmentation error in mex file

Question

Here is the mex code I wrote to read in a tab-delimited file. The mex file got created but it causes my MATLAB to end abruptly and give the following error. Can anyone help me where I am going wrong? Please let me know if any further information is required

Abnormal termination: Segmentation violation

#include "mex.h"
#include "matrix.h"
#include <stdio.h>
#include<string.h>
#include<stdlib.h>  

void mexFunction(int nlhs,mxArray *plhs[],int nrhs,const mxArray *prhs[])
{   

    FILE *ptr_file;
    const char **field_names;       /* pointers to field names */
    char *buf[1024];
    char *temp[20];
    int count;
    int i, j, k, l;
    int date_field, mva_field, qc_load_field, air_field, qc_air_field, oil_field, qc_oil_field, wind_a_field, qc_wind_a_field, wind_b_field, qc_wind_b_field, wind_c_field, qc_wind_c_field, tamb1_field, qc_tamb1_field;
    char *NAME;
    NAME=mxArrayToString(prhs[0]);
    count = 0;
//open file to count elements   
    ptr_file =fopen(NAME,"r");
    if (ptr_file != NULL)
    {
//skip first 3 lines    
    fgets(buf, sizeof(buf), ptr_file);
    fgets(buf, sizeof(buf), ptr_file);
    fgets(buf, sizeof(buf), ptr_file);
//start counting no. of elements    
    while(fgets(buf, sizeof(buf), ptr_file) != NULL)
        count++;
    fclose(ptr_file);
    }
        field_names[0] = "date";
    field_names[1] = "mva";
    field_names[2] = "qc_load";
    field_names[3] = "air";
    field_names[4] = "qc_air";
    field_names[5] = "oil";
    field_names[6] = "qc_oil";
    field_names[7] = "wind_a";
    field_names[8] = "qc_wind_a";
    field_names[9] = "wind_b";
    field_names[10] = "qc_wind_b";
    field_names[11] = "wind_c";
    field_names[12] = "qc_wind_c"; 
    field_names[13] = "tamb1";
    field_names[14] = "qc_tamb1"; 

    plhs[0] = mxCreateStructMatrix(count, 1, 15, field_names);
    plhs[1] = mxCreateDoubleMatrix(1,1,mxREAL);
    date_field = mxGetFieldNumber(plhs[0],"date");
    mva_field = mxGetFieldNumber(plhs[0],"mva");
    qc_load_field = mxGetFieldNumber(plhs[0],"qc_load");
    air_field = mxGetFieldNumber(plhs[0],"air");
    qc_air_field = mxGetFieldNumber(plhs[0],"qc_air");
    oil_field = mxGetFieldNumber(plhs[0],"oil");
    qc_oil_field = mxGetFieldNumber(plhs[0],"qc_oil");
    wind_a_field = mxGetFieldNumber(plhs[0],"wind_a");
    qc_wind_a_field = mxGetFieldNumber(plhs[0],"qc_wind_a");
    wind_b_field = mxGetFieldNumber(plhs[0],"wind_b");
    qc_wind_b_field = mxGetFieldNumber(plhs[0],"qc_wind_b");
    wind_c_field = mxGetFieldNumber(plhs[0],"wind_c");
    qc_wind_c_field = mxGetFieldNumber(plhs[0],"qc_wind_c");
    tamb1_field = mxGetFieldNumber(plhs[0],"tamb1");
    qc_tamb1_field = mxGetFieldNumber(plhs[0],"qc_tamb1");


//open file again for storing elements columnwise
    ptr_file =fopen(NAME,"r");
    if (ptr_file != NULL)
    {
//skip first 3 lines    
    fgets(buf, sizeof(buf), ptr_file);
    fgets(buf, sizeof(buf), ptr_file);
    fgets(buf, sizeof(buf), ptr_file);  
//start collecting data 
    for(i=0;i<count;i++){   //increment line
        //get line
        fgets(buf, sizeof(buf), ptr_file);
        j=0;
        k=0;
        //extract first word
        while(buf[j] != '\t'){
            temp[k] = buf[j];
            j++;
            k++;
        }
        temp[k] = '\0';
        j++;
        mxSetFieldByNumber(plhs[0],i,date_field,mxCreateString(temp));
//      strcpy(elem[i].date, temp);

        //extract second word
        k=0;
        while(buf[j] != '\t'){
            temp[k] = buf[j];
            j++;
            k++;
        }
        temp[k] = '\0';
        j++;
//      elem[i].mva = atof(temp);
        *mxGetPr(plhs[1]) = atof(temp);
        mxSetFieldByNumber(plhs[0],i,mva_field,plhs[1]);    

        //extract third word
        k=0;
        while(buf[j] != '\t'){
            temp[k] = buf[j];
            j++;
            k++;
        }
        temp[k] = '\0';
        j++;
//      strcpy(elem[i].qc_load, temp);  
        mxSetFieldByNumber(plhs[0],i,qc_load_field,mxCreateString(temp));
// similarly for other fields of the structure. 
    fclose(ptr_file);
    }
}

Answer 1

You're getting a segfault because rather than field_names pointing to somewhere you can treat as an array of const char *, it points nowhere because it's uninitialised. Hence

const char **field_names;    /* pointer to nowhere */
...
field_names[0] = "date";     /* dereferences invalid pointer and BANG */

What you want is actually:

const char *field_names[15]; /* array of pointers to const char */
...
field_names[0] = "date";     /* all is well and good */
...

Key C fact: pointers are not arrays and arrays are not pointers, but in some circumstances an array will be implicitly converted to a pointer to its first element - such that it is still correct to pass this field_names array to mxCreateStructMatrix() in place of a const char **.

---

However, fix that and you'll then hit the problems with this:

plhs[1] = mxCreateDoubleMatrix(1,1,mxREAL);

Firstly, since you haven't checked that nlhs is at least 2, plhs[1] may well not exist and similarly blow up when you try to set its value. IIRC, setting plhs[0] without checking is probably OK, since even if nlhs == 0, plhs[0] should point to Ans instead of an explicit output variable, but really it's good practice to check nlhs before doing anything and act accordingly. Similarly using prhs[0] without checking nrhs > 0 isn't advisable.

Secondly, later on you'll end up assigning this same mxArray to multiple fields of multiple structures which is not a valid thing to do (and may cause Matlab to crash later due to heap corruption). Even if it were allowed I don't think you want every numeric field in the whole structure array pointing to one single double that's been overwritten n-thousand times...

Third, I don't quite understand the use of plhs[1] at all, unless you really do want a mandatory second output of "whatever the last number read was". What I suspect you were going for is this:

mxArray *mxtmp;
...
    /* in parsing loop */
    mxtmp = mxCreateDoubleScalar(atof(temp));
    mxSetFieldByNumber(plhs[0], i, mva_field, mxtmp); 

---

Elsewhere, whilst indeed you can store char values in pointers, it's not a sane or meaningful thing to do and in this case is clearly another mistake:

char *buf[1024];  /* array of pointers to char - wrong */
char *temp[20];

char buf[1024];   /* array of char - right */
char temp[20];