Hi, ok I tried to make the vts dynamic. I made the most part, but I have some segfaults, and I am not sure why yet. I have some at least on the line: region = XCreateRegion(); in rxvt_tabbar_expose (tabbar.c:1933). [If I set a debug message before and one after this function, the segfault arrives before the 2nd message, so...] This happens in this exposure function, apparently only when I remove a tab.
I don't understand why yet, as this is a X function and that before there was no such issue, as far as I know. So there must be some relation with the dynamic vts array. I am a little tired now and must go to sleep, but I have committed anyway (it is compiling and running...). I hope this is OK. And if ever there is a real issue about this, the last revision without this dynamic array which segfaults was 329. I will continue to work on this tomorrow. See you. Jehan jehan a écrit : > Gautam Iyer writes: >> Hi Jehan, >> >> Maybe. What needs to be kept in mind are the following: >> >> 1. The size is only about 4k. If the amount of code required to deal >> with a dynamic buffer here is comparable then it is probably not >> worth it. >> >> 2. As far as I remember, the input buffer grows when needed, but >> does not shrink when too big. This should of course be avoided. >> >> Given the small size of these arrays, and the short amount of developer >> resources we have perhaps it would be better to leave these as static. >> I'd go so far as to suggesting we convert the v_buffer's to (small) >> static arrays... >> > > Ok. Obviously it is not really prioritary, you are right. I will see if it > is worth it. But I am not sure that making v_buffer static as well is a good > idea. > >> If you're willing to implement it, it should improve Mrxvt's memory >> usage. It would also be good to add some code reducing the size of >> v_buffer when it's too big, >> > > There is already some code for v_buffer. In rxvt_tt_write, from command.c, > function which writes data in this buffer, at the end of the function: if > the free space in the physical buffer is 8 times greater than MAX_PTY_WRITE > (which equals 128 in our code and is the maximum of bytes we will write in > one time in the real child's file descriptor from this buffer), then we > reallocate the buffer to the smaller multiple of MAX_PTY_WRITE which can > contain the current data. > So here this is not about proportional size of the real data compared to the > buffer size, but about the empty size compared to some given value... > >> Re-allocing here would be totally OK. A new tab is created very >> infrequently (as opposed to typing / reading data). So a rather 'stupid' >> realloc every time a new tab is created / closed should be OK. >> > > I will consider this so. :-) > Anyway my conclusion of this is that "I will see" and then I will tell you. > :-) > > But I have one last (related) point: this is about naming. Would you care if > I try to rename some of the variables (when it is not too difficult, or > really worth it)? For instance here, we have 2 variables which are like > "symetric" (one is the output, the other the input of the same fd) and here > is the naming: > > - The start of the physical buffer is v_buffer on one hand, cmdbuf_base on > the other. > - The start of real data is v_buffstr and cmdbuf_ptr. > - The end of data is v_bufptr (notice that "ptr" was used to indicate the > beginning for cmdbuf, and here the end!) and cmdbuf_endp. > - The end of physical buffer is v_bufend (and there is no end for cmdbuf as > it is static, so always cmbuf_base + BUFSIZ). > > This is absolutely illogical and unconsistant, so pretty perturbating. > 1/ The base name: v_buf and cmdbuf. I am sure there has been a reason > (probably a technical one, which is not the best way I think, because > technics can change with time, but notions less) the first time it was > named. But now when I read the code. Even though I know it well enough, I am > still unsure sometimes and check which one is which one. I think a name more > "functional"/meaningful would help better developpers. Something simple as > "input_buffer" or "output_buffer", or smaller if you prefer: inbuf and > outbuf. > > 2/ The suffixes should be consistant. Like "_base" for the physical buffer > is ok; and "_start" maybe for the beginning of data? Or maybe _data, or even > stay with _str or _ptr if you prefer (as long as we stay consistent); _end > is good for end of data; and for end of buffer, I don't know, but maybe > could it be replaced by the buffer's size (we never need a pointer to the > end of the buffer as nothing is allocated there anyway! But at the opposite > we often need the size of the buffer and we calculate each time the end > minus the start. So let's avoid useless calculation...). > > Change these is really not difficult because it can be done by a simple > string exchange in all files (and they are not used so much anyway) and some > basic checks. But I think it can be very good for the understanding of code > for further devs. > > Thanks. > > Jehan > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Materm-devel mailing list > Materm-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/materm-devel > mrxvt home page: http://materm.sourceforge.net > > ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Materm-devel mailing list Materm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/materm-devel mrxvt home page: http://materm.sourceforge.net
