0

I want to set mixer device from gtk_entry with this form "/dev/mixer:line" or "/dev/mixer:cd".

User must to entry in this format mixer device settings:

/dev/mixer:line

or:

/dev/mixer:cd

For this I write code to setup mixer and have same dilemma with strdup() function. It is wrong to free() char with assigned value before call strdup()?

char *mixer_device = "/dev/mixer";
int mixer_channel = SOUND_MIXER_LINE;
int fd = -1;

int get_volume( void )
{
    int v, cmd, devs;
    int curvol = 0;

    if( fd < 0 ) fd = open( mixer_device, O_RDONLY );
    if( fd != -1 ) {

            ioctl( fd, SOUND_MIXER_READ_DEVMASK, &devs );
            if( devs & mixer_dev_mask ) {
                    cmd = MIXER_READ( mixer_channel );
            } else {
                    return curvol;
            }

            ioctl( fd, cmd, &v );
            curvol = ( v & 0xFF00 ) >> 8;
    }

    return curvol;
}

char *core_devnames[] = SOUND_DEVICE_NAMES;

int set_device( const char *devname )
{
    const char *channame;
    int i;

    /* if (mixer_device) free (mixer_device) <-- It is wrong ??? */ 
    mixer_device = strdup( devname );
    if( !mixer_device ) return -1;

    i = strcspn( mixer_device, ":" );
    if( i == strlen( mixer_device ) ) {
            channame = "line";
    } else {
            mixer_device[ i ] = 0;
            channame = mixer_device + i + 1;
    }
    fd = open( mixer_device, O_RDONLY );
    if( fd == 0 ) {
            fprintf( stderr, "mixer: Can't open device %s, "
                     "mixer volume and mute unavailable.\n", mixer_device );
            return -1;
    }

    return 0;
}

It is wrong to free() char with assigned value before call strdup()

user1840007
  • 615
  • 1
  • 10
  • 19
  • 3
    Calling `free` on non dynamically allocated memory is undefined behavior. You set the initial value to a string literal here: `char *mixer_device = "/dev/mixer";` – Shafik Yaghmour Jul 29 '13 at 17:09

3 Answers3

1

You may only call free() if the pointer points to memory that was allocated with malloc(). In your program, mixer_device initially points to a literal string, so calling free() would cause undefined behavior.

Instead of checking whether mixer_device is null, you need another variable that keeps track of whether it points to the initial literal string or a new string created with strdup(). Or, instead of pointing it to a literal string, your startup code could do:

mixer_string = strdup("/dev/mixer");

so it's always safe to free it.

Barmar
  • 741,623
  • 53
  • 500
  • 612
0

You set the initial value here:

char *mixer_device = "/dev/mixer";

It now points to a string literal, so if you try to call free on mixer_device you will have undefined behavior, free can only be called on dynamically allocated memory, i.e. from malloc, strdup etc... One solution would be to use strdup to initialize your variable:

mixer_device = strdup("/dev/mixer") ;
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
0

Initialize with NULL;

char *mixer_device = NULL;

It is OK to use free(mixer_device) when mixer_device has the value NULL or, of course, a value assigned through malloc(), strdup(), realloc(). wHEN null, Nothing gets freed and no UB. Then when ever you want to assign mixer_device, simply

free(mixer_device);
mixer_device = strdup(NewName);

At the end main(), perform a final

free(mixer_device);
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256