[linux-dvb] [PATCH/RFC] add dvb-s2 support to frontend.h and dvb_frontend.[h|c]

[Philip Prindeville]

Alan Nisota wrote:

>On 4/15/06, Alan Nisota <alannisota at gmail.com> wrote:
>  
>
>>The issue I have is
>>that set_frontend is called from dvb_frontend_swzigzag_autotune and
>>dvb_frontend_swzigzag both of which examine fepriv->parameters.  Do
>>you expect to update these functions to use fepriv->parameters_new (I
>>assume so)?  if so, how do you know when to use one vs the other?
>>    
>>
>
>I made some assumptions about your intentions and completed the
>implementation in dvb_frontend.c.  I then finished off the driver, but
>I haven't been able to test yet, as I still need to update mythtv to
>support the new ioctls.
>
>I hope this is approximately what you intended.
>
>Index: v4l-dvb/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>===================================================================
>--- v4l-dvb.orig/linux/drivers/media/dvb/dvb-core/dvb_frontend.c      
> 2006-04-15 13:07:55.000000000 -0700
>+++ v4l-dvb/linux/drivers/media/dvb/dvb-core/dvb_frontend.c    
>2006-04-15 13:07:47.000000000 -0700
>@@ -272,7 +272,7 @@
>        /* are we using autoinversion? */
>        autoinversion = ((!(fe->ops->info.caps & FE_CAN_INVERSION_AUTO)) &&
>                ((fepriv->current_standard_set &&
>-                 fepriv->parameters_new.inversion == INVERSION_AUTO) ||;
>+                 fepriv->parameters_new.inversion == INVERSION_AUTO) ||
>                 (! fepriv->current_standard_set &&
>                   fepriv->parameters.inversion == INVERSION_AUTO)));
>
>@@ -996,12 +996,22 @@
>                        break;
>                }
>
>+               if (fepriv->parameters_new.u)
>+                       kfree(fepriv->parameters_new.u);
>  
>

Isn't kfree() NULL-safe?  Also, shouldn't the pointer be nulled out to avoid
a double-free?  Oh, never mind.  You're about to overwrite the pointer
lower down...


>+
>                castedparg = (struct dvb_frontend_parameters_new *)parg;
>                memcpy (&fepriv->parameters_new, parg,
>                         sizeof (struct dvb_frontend_parameters_new));
>  
>

Instead of sizeof (struct foo) you could also have sizeof(*parg) ... as
long as parg
isn't a pointer to a char.


>                memset(&fetunesettings, 0, sizeof(struct
>dvb_frontend_tune_settings));
>  
>

Ditto...  but no point in changing this now to sizeof(fetunesettings)...

>+               fepriv->parameters_new.u = (struct
>frontend_parameters_union *) kmalloc(castedparg->size, GFP_KERNEL);
>+               if ( !fepriv->parameters_new.u ) {
>+
>+                       err = -ENOMEM;
>+                       break;
>+               }
>+               memcpy(fepriv->parameters_new.u, castedparg->u,
>castedparg->size);
>                /* be sure struct union pointer is set - even if it is NULL */
>                fetunesettings.parameters_new.u = (struct
>frontend_parameters_union *) kmalloc(castedparg->size, GFP_KERNEL);
>                if ( !fetunesettings.parameters_new.u ) {
>@@ -1066,7 +1076,13 @@
>                                printk("dvb-core: FE_ATSC not handled yet.\n");
>                                break;
>                        case FE_DVB_S2:
>-                               printk("dvb-core: FE_DVB_S2 not
>handled yet.\n");
>+                               if (fepriv->current_standard != FE_DVB_S2) {
>+                                       err = -EINVAL;
>+                                       break;
>+                               }
>+                               fepriv->min_delay = HZ/20;
>  
>

Spaces around "/".


>+                               fepriv->step_size =
>fepriv->parameters_new.u->qpsk2.symbol_rate / 16000;
>+                               fepriv->max_drift =
>fepriv->parameters_new.u->qpsk2.symbol_rate / 2000;
>                                break;
>
>                        }
>  
>