passing argument 1 of 'strlen' makes pointer from integer without a cast

I write code witch generate file name based on: destination directory settings, radio station presets list and current time.

If users store radio station name (app presets) with white space in name:

Europa FM
Paprika Radio
Radio France Internationale
...........................
Rock FM

I want to remove whitespace for recording radio station filename and result look like this:

/home/ubuntu/Desktop/EuropaFM_07042012-111705.ogg

Code:

---

static int start_recording(const gchar *destination, const char* station, const char* time)
{
    Recording* recording;
    char *filename;

    char* remove_whitespace(station)
    {
        char* result = malloc(strlen(station)+1);
        char* i;
        int temp = 0;
        for( i = station; *i; ++i)
            if(*i != ' ')
            {
                result[temp] = (*i);
                ++temp;
            }
        result[temp] = '\0';
        return result;
    }

    filename = g_strdup_printf(_("%s/%s_%s"),
        destination,
        remove_whitespace(station),
        time);
    recording = recording_start(filename);
    g_free(filename);
    if (!recording)
        return -1;

    recording->station = g_strdup(station);
    tray_icon_items_set_sensible(FALSE);

    record_status_window(recording);

    run_status_window(recording);

    return 1;
}

---

But this get warnings:

warning: passing argument 1 of 'strlen' makes pointer from integer without a cast [enabled by default] warning: assignment makes pointer from integer without a cast [enabled by default]

I appreciate your help. Thanks.

O_o

Missing ';' can cause unusual problems...

Soma

Please explain the following snippet:

Code:

---

char* remove_whitespace(station)
    {
        char* result = malloc(strlen(station)+1);
        char* i;
        int temp = 0;
        for( i = station; *i; ++i)
            if(*i != ' ')
            {
                result[temp] = (*i);
                ++temp;
            }
        result[temp] = '\0';
        return result;
    }

---

To me this looks like a function implementation. inside another function, using a default integer parameter. Which should explain the error message.

Several problems, one never implement a function inside a function. Two never use default parameters when defining, implementing functions. Three you probably have a memory leak because you aren't assigning the value being returned to any variable.


Jim

Quote:

---

Originally Posted by jimblumberg

Please explain the following snippet:

Code:

---

char* remove_whitespace(station)
    {
        char* result = malloc(strlen(station)+1);
        char* i;
        int temp = 0;
        for( i = station; *i; ++i)
            if(*i != ' ')
            {
                result[temp] = (*i);
                ++temp;
            }
        result[temp] = '\0';
        return result;
    }

---

To me this looks like a function implementation. inside another function, using a default integer parameter. Which should explain the error message.

Several problems, one never implement a function inside a function. Two never use default parameters when defining, implementing functions. Three you probably have a memory leak because you aren't assigning the value being returned to any variable.


Jim

---

Maybe solution is to remove whitespace here?

Code:

---

void rec_button_clicked_cb(GtkButton *button, gpointer app)
{
    char *station;
    char time_str[100];
    time_t t;
   
    t = time(NULL);
    /* consult man strftime to translate this. This is a filename, so don't use "/" or ":", please */
    strftime(time_str, 100, _("%m%d%Y-%H%M%S"), localtime(&t));
   
    if (mom_ps < 0) {
        station = g_strdup_printf(_("%.2f MHz"), rint(gtk_adjustment_get_value(adj))/STEPS);
    } else {
        g_assert(mom_ps < g_list_length(settings.presets));
        preset* ps = g_list_nth_data(settings.presets, mom_ps);
        g_assert(ps);
   
        station = g_strdup(ps->title);
    }

---

Quote:

---

Maybe solution is to remove whitespace here?

---

I don't know, maybe here, maybe there. That is a decision you need to make. I didn't see anything wrong about where you were trying to remove the whitespace in your original post, just the method used. As to where you remove the whitespace, that is a design decision that you the programmer must make.

Jim

Because you have not specified the type of parameter station in the remove_whitespace function declaration, it defaults to an int.

I'd recommend using a simple file name sanitizer instead, which also prepends the specified directory name (NULL if none) to yield the full path to the target file.

Code:

---

#include <stdlib.h>
#include <string.h>
#include <errno.h>

static inline char *path_to(const char *const dir, const char *const name)
{
    const size_t  dirlen = (dir) ? strlen(dir) : 0;
    const size_t  namelen = (name) ? strlen(name) : 0;
    char        *result;
    const unsigned char *s;
    unsigned char *d;

    /* Empty name is not allowed. */
    if (namelen < 1) {
        errno = EINVAL;
        return NULL;
    }

    result = malloc(dirlen + namelen + 4);
    if (!result) {
        errno = ENOMEM;
        return NULL;
    }

    d = (unsigned char *)result;

    if (namelen > 0) {
        s = (const unsigned char *)dir;

        if (*s != '/')
            *(d++) = '.';

        while (1) {

            while (*s == '/')
                s++;

            if (!*s)
                break;

            *(d++) = '/';

            while (*s && *s != '/')
                *(d++) = *(s++);
        }

      *(d++) = '/';
    }

    s = (const unsigned char *)name;

    while (*s)
        if (*s <= 32 || *s == 47 || *s == 127)
            s++; /* Skip whitespace, control chars, and slashes */
        else
            *(d++) = *(s++);

    /* Terminate path. */
    *d = '\0';

    /* Empty result? */
    if ((char *)d == result) {
        free(result);
        errno = EINVAL;
        return NULL;
    }

    /* Empty file name part? */
    if (*(d - 1) == '/') {
        free(result);
        errno = EINVAL;
        return NULL;
    }

    return result;
}

---

The above one does not sanitize the directory part, only the file name part.

ASCII control characters, ASCII whitespace, and slashes (/) are removed from the file name part.

If a path can be constructed safely, it is returned as a dynamically allocated string. After you no longer need it, you must remember to free() the returned pointer.

If the function cannot construct a path to a file, it will return NULL, with errno set to indicate the reason: ENOMEM if out of memory, and EINVAL if the file name part is empty.

It is safe to call the function with NULL parameters. The directory may be NULL; the path generated will then have no directory part. (If the directory part is empty, the result will start with ./, i.e. current directory). The file name part must not be empty, or be empty after sanitization, or the function will return NULL with errno set to EINVAL.

In your particular case, I'd use

Code:

---

/* Instead of 'time' to the function, pass a pointer to GDateTime: */
GDateTime *now;

g_char *temptime, *tempname;
char *path;

temptime = g_date_time_format(_("%x %X"), now);
if (!temptime)
    return 0; /* Out of memory, or localization error */

tempname = g_strdup_printf(_("%1$s_%2$s"), station, temptime);
g_free(temptime);
if (!tempname)
    return 0; /* Out of memory, or localization error */

path = path_to((const char *)destination, (const char *)tempname);
if (!path) {
    /* errno == EINVAL: Invalid file name (possibly bad localization),
    * errno == ENOMEM: Out of memory.
    */
    g_free(tempname);
    return 0; /* Out of memory, invalid file name, or localization error */
}
g_free(tempname);

/* Record to file 'path' ... */

free(path);
return 1; /* Success */

---

This way the way time is used in the file names is localizable. It should default to full date and time according to the current locale. Also, the way the station name and date-time is combined to form the file name is localizable. Whatever we get after the two localization steps, we feed through the function I showed above, to construct the full path to the target file.

For example, if I localized that to Finnish, I'd use YYYYMMDD-hhmm-station by using %Y%m%d-%H%M and %2$s-%1$s in the .po file.

Note how the above calls g_free() and free() to discard the dynamically allocated temporaries when they're no longer used. It will not leak memory even if an error occurs.

(For future reference: It is very useful to remember that both g_free(NULL) and free(NULL) are safe; they do nothing. If you start getting complicated if clauses at the error conditions, just initialize all dynamically allocated temporaries to NULL, and in all exit cases just free them all unconditionally. Elsewhere, when you free something, also assign the pointer to NULL. It is safe, and yields quite readable code, although some programmers complain about how the "superfluous" free()s and g_free()s hurt their eyes or something.)