Wind_of_Change Report post Posted March 17, 2012 (edited) When looking at elconfig.c I was kind of wondering why there is the need to use queues to store the options. I thought queues were something you could use to push and pop stuff in a thread safe way (for example for messages). Here they are used to store min/max values or options. Is their complexity really needed here? I mean for example you need to initialize them and clean them up in the end. Wouldn't it be easier to use a union like: typedef struct { option_type type; /*!< type of the variable */ char *name; /*!< name of the variable */ int nlen; /*!< length of the \a name */ char *shortname; /*!< shortname of the variable */ int snlen; /*!< length of the \a shortname */ void (*func)(); /*!< routine to execute when this variable is selected. */ void *var; /*!< data for this variable */ union { int len; //For OPT_STRING and OPT_PASSWORD struct { float min; float max; float interval; } f; //For OPT_FLOAT struct { int min; int max; } i; //For OPT_INT and OPT_INT_INI struct { int len; char **options; } m; //For OPT_MULTI and OPT_MULTI_H struct { float (*min)(); float (*max)(); float interval; } f_f; //For OPT_FLOAT_F struct { int (*min)(); int (*max)(); } i_f; //For OPT_INT_F } data; int saved; // char *message; /*!< In case you want a message to be written when a setting is changed */ dichar display; struct { int tab_id; /*!< The tab ID in which we find this option */ int label_id; /*!< The label ID associated with this option */ int widget_id; /*!< Widget ID for things like checkboxes */ } widgets; } var_struct; Edited March 17, 2012 by Wind_of_Change Share this post Link to post Share on other sites
ago Report post Posted March 17, 2012 When looking at elconfig.c I was kind of wondering why there is the need to use queues to store the options. I thought queues were something you could use to push and pop stuff in a thread safe way (for example for messages). Here they are used to store min/max values or options. Is their complexity really needed here? I mean for example you need to initialize them and clean them up in the end. A queue is a dynamic datastructure that allows adding items to the end (push, enque) and removing items at the front (pop, deque). In EL it's implemented with a linked list. Once that is implemented there's not much complexity besides allocation, push and pop and deallocating it somewhere. Most of the options don't need the dynamic length but MULTI_H does. You could have the same result with a list or a dynamic char* array but you will have to allocate and deallocate the array and do the array length checks too. It does not sound like there is much gained by using a dynamicly allocated array. Share this post Link to post Share on other sites
Wind_of_Change Report post Posted March 17, 2012 A queue is a dynamic datastructure that allows adding items to the end (push, enque) and removing items at the front (pop, deque). In EL it's implemented with a linked list. Once that is implemented there's not much complexity besides allocation, push and pop and deallocating it somewhere. Apart from the multithread stuff. But its hidden in queue.c anyway. Most of the options don't need the dynamic length but MULTI_H does. You could have the same result with a list or a dynamic char* array but you will have to allocate and deallocate the array and do the array length checks too. It does not sound like there is much gained by using a dynamicly allocated array. In MULTI, too? As far as I see it dynamical feature is only used for the fonts. (Isn't actually the number of fonts artificially restricted by FONTS_ARRAY_SIZE?) But isn't is kind of inappropriate to store only indices for multioptions (which might be variable) in the config file? Share this post Link to post Share on other sites
ago Report post Posted March 17, 2012 As far as I see it dynamical feature is only used for the fonts. (Isn't actually the number of fonts artificially restricted by FONTS_ARRAY_SIZE?) The fsaa feature dynamicly adds the supported anti aliasing modes. The list of video modes is added dynamicly to the queue. It's possible to have the array declared with the proper size to allow all predefined video mode and FSAA options. But to be honest, using a generic list or queue sounds much better than using an array with a fixed size that depends on the size of two other arrays somewhere else. But isn't is kind of inappropriate to store only indices for multioptions (which might be variable) in the config file? Sure. The FSAA indices may have different meanings if a different set of FSAA modes is supported like after a driver update. But this is not related to using a queue or a fixed lenth array. Share this post Link to post Share on other sites
Wind_of_Change Report post Posted March 17, 2012 (edited) Thanks for your answers. The fsaa feature dynamicly adds the supported anti aliasing modes. The list of video modes is added dynamicly to the queue. It's possible to have the array declared with the proper size to allow all predefined video mode and FSAA options. But to be honest, using a generic list or queue sounds much better than using an array with a fixed size that depends on the size of two other arrays somewhere else. I saw these things in the code as well and agree with you that such things should be added dynamically at runtime. Queues do what is wanted hiding some complexity, reallocs for every addition does work but not seem good for those. But how about a new type especially for those dynamic variables needing additional data like: struct { int (*len)(); char* (*str)(int i); }; That is even more dynamic than the queue approach. The corresponding fsaa functions do already exist! (EDIT: Not quite, the functions work a bit different than I expected) Ok, but I was actually thinking about something more radical, when looking at the magnitude of add_var calls. That is having a gigantic constant array instead, for which there is no need to initialise at runtime. (Only the widgets would need to be initialised, but that can happen quite late in the loading of the client.) Since the variables are only used indirectly outside elconfig.c that would involve only (huge) changes in that file. But isn't is kind of inappropriate to store only indices for multioptions (which might be variable) in the config file? Sure. The FSAA indices may have different meanings if a different set of FSAA modes is supported like after a driver update. But this is not related to using a queue or a fixed lenth array. Of course your right. Wouldn't storing this kind of stuff in form of strings solve this "problem"? (Creating string parsing complications...) Edited March 18, 2012 by Wind_of_Change Share this post Link to post Share on other sites
nathanstenzel Report post Posted March 18, 2012 struct { int (*len)(); char* (*str)(int i); }; C and C++ (EL uses normal C, if I recall correctly) are monsters when it comes to memory management. If you do not allocate and deallocate your memory very carefully, it will be big problems. You can't try to access memory past the end of an allocated memory block without it being disastrous. Since the queue is a well planned out class, it probably has safeguards against trying to access data past the end and stuff like that. I have not looked at the code for some time and you may actually already know what I just said, but I figured it had to be said anyways. I don't know if EL actually clears the items out of the queue during loadup or if it waits until the client is closed. Share this post Link to post Share on other sites
Wind_of_Change Report post Posted March 19, 2012 C and C++ (EL uses normal C, if I recall correctly) are monsters when it comes to memory management. If you do not allocate and deallocate your memory very carefully, it will be big problems. You can't try to access memory past the end of an allocated memory block without it being disastrous. Since the queue is a well planned out class, it probably has safeguards against trying to access data past the end and stuff like that. EL uses some combination of C and C++. I don't think "monster" is the right word^^ You just need to manage the memory yourself. In the queue approach, pointers for the multi options are put on the queue and poped then to give them to the multi option widget (which makes copies of them). In the function approach the functions will only be called for the creation of the multi option widget. In both cases the allocated strings have to be freed in the end. (for example in cleanup_mem in main.c) Those queues are also misused to store 3 floats... I see the point for the multi_options though. Is the "dichar display" actually intended to use for localization? Share this post Link to post Share on other sites
