Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
mpegts mux dvb: fix the load sequence and possible memory leak
  • Loading branch information
perexg committed Jan 27, 2015
1 parent 3c0a279 commit c664af6
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions src/input/mpegts/mpegts_mux_dvb.c
Expand Up @@ -649,11 +649,20 @@ dvb_mux_create0
}

/* Create */
if (!(mm = mpegts_mux_create0(calloc(1, sizeof(dvb_mux_t)), idc, uuid,
(mpegts_network_t*)ln, onid, tsid, conf)))
return NULL;
mm = calloc(1, sizeof(dvb_mux_t));
lm = (dvb_mux_t*)mm;

/* Defaults */
lm->lm_tuning.dmc_fe_inversion = DVB_INVERSION_AUTO;
lm->lm_tuning.dmc_fe_pilot = DVB_PILOT_AUTO;

/* Parent init and load config */
if (!(mm = mpegts_mux_create0(mm, idc, uuid,
(mpegts_network_t*)ln, onid, tsid, conf))) {
free(mm);
return NULL;
}

/* Tuning */
if (dmc)
memcpy(&lm->lm_tuning, dmc, sizeof(dvb_mux_conf_t));
Expand All @@ -665,12 +674,6 @@ dvb_mux_create0
lm->mm_config_save = dvb_mux_config_save;
lm->mm_create_instances = dvb_mux_create_instances;

/* Defaults */
if (dmc == NULL) {
lm->lm_tuning.dmc_fe_inversion = DVB_INVERSION_AUTO;
lm->lm_tuning.dmc_fe_pilot = DVB_PILOT_AUTO;
}

/* No config */
if (!conf) return lm;

Expand Down

2 comments on commit c664af6

@treitmayr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the code in line 660 supposed to work? If mpegts_mux_create0 returns NULL then this is assigned to mm and inside the if is then freed?
Also I did not see any memory leak as mpegts_mux_create0 frees mm anyway in its single error path.
However, maybe I misunderstood something...

@EricV
Copy link
Contributor

@EricV EricV commented on c664af6 Jan 31, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could also argue that calloc return value is unchecked and dereferenced. I know that this means system is going to die via OOM killer anyway but still

Please sign in to comment.