On Wednesday 08 August 2007 05:47, John Drescher wrote:
> > That is a really interesting idea that I doubt that I would have thought
> > of, and I would have worried a lot about the GTK++ code. However, I
> > think you may be on to something really good here -- it is always good to
> > leverage the software so that the same code base can be ported to
> > multiple platforms. It reduces the volume of code and reduces bugs too.
>
> After examining how the win32 bacula-dir, bacula-fd and bacula-sd
> worked I decided to take that path even though it would be a lot
> harder for my task. Also you did mention you wanted to extended the
> unix tray as well.
Yes, it could make the initial phase a bit more difficult, but it has the
advantage that it already has the config and the comm code integrated, which
is a large part of the project.
>
> > > So now I
> > > have begun to port the parts of the code that I disabled. The first
> > > thing I see is that if I get rid of GString and replace it with a dlist
> > > of dlistString that would be a big start to the port. The problem is
> > > that the functions create the string list from inside and pass it back
> > > to the caller using the ** convention.
> >
> > Hmm. Yes, in looking at the code, IMO, it does it exactly backwards.
>
> I agree with you here. I was thinking of changing how the code
> worked to fix that but at first pass I was thought it might be better
> to make as few design changes as possible while keeping the current
> unix functionality.
Yes, I agree that making as few design changes as possible is a much better
way to go, and the way I would approach it. However, in this case, the
design change is probably not too destabilizing (said without looking at the
code). You might start by allocating the GString list at the top level,
making sure it works, then converting to a dlist. One thing I forgot in
the schematic I sent you was to do the delete of the dlist structure :-)
>
> > I really
> > try to avoid calling a subroutine that allocates memory then passes it
> > back expecting the higher level subroutine to release the memory. It is
> > a constant source of memory leaks. It would probably be better to invert
> > the process and allocate the string list in the main routine where it
> > would also be freed. In addition, since we are working in C++, it would
> > be much better to pass the list with GSList &list;
>
> That is what I would have done if I was writing my own code but I was
> unsure of how much if at all this was done in the current bacula
> source code.
Well, since Bacula started as a C program there is not much use of pass by
reference. In most cases I discourage it because it means that subroutines
have "side effects" that is the subroutine can modify structures of the
calling program (pass by reference). So where I can easily avoid it, I do,
but in many cases, one really wants the subroutine to modify a variable in
the calling program and I find passing by reference preferable to passing a
pointer to a pointer (**xxx).
>
> > rather than GSList **list; or if it is
> > allocated and released in the main part of the code, it could even be
> > passed as GSList *list rather than GSList **list.
> >
> > > Basically I would like to convert
> > >
> > > int docmd(monitoritem* item, const char* command, GSList** list);
> > >
> > > to
> > >
> > > int docmd(monitoritem* item, const char* command, dlist** list)
> > >
> > >
> > > So the question is how does one create and initialize a dlist* that
> > > contains dlistStrings. Being that I am a C++ programmer I would use new
> > > as I have done thousands of times but I see that the bacula code is
> > > using New() which basically allocates the memory in using c calls and
> > > then initializes it.
> >
> > Here you have stumbled on one of the more non-standard features of
> > Bacula. I don't particularly like most C++ constructs and especially new
> > xxx. The problem with new is that there is no good way to interpose code
> > such as Bacula's smartalloc, which checks for orphaned buffers (memory
> > leaks), double frees, and corrupted heap. The solution a C++ programmer
> > came up with was the New() construct, which I have decided to eliminate
> > slowly but surely in favor of a C like interface -- e.g.
> > new_dlistString("xxx");
>
> I see. I did notice smartalloc was being used and it helped trak down
> memory leaks but I did not know it did all that.
Yes, smartalloc is a lifesaver. It is a *critical* feature of a program like
Bacula that must run for months. The only place where Bacula has ever had
memory leaks is in 3rd party software such as MySQL libraries where Bacula
cannot use smartalloc -- I am forced to periodically running valgrind.
By the way, when using Bacula memory allocation AND GTK+ or wxWidgets
subroutines that malloc(), you need to be *very* careful not to mix the two.
With GTK+, the situation is not too bad because one normally does not do a
free() of a GTK+ item. If memory was allocated by GTK+ and you do a free()
in Bacula code, it will blow up because smartalloc will not recognize the
memory being freed. If you *really* need to free something allocated in a
library use actuallyfree() -- I discourage this, but every now and then it is
necessary.
>
> > In addition, I don't like templates. 5 years ago, they were not at all
> > portable. This means that all the list routines that Bacula uses are
> > written using offsets to allow handling links, and using void to handle
> > multiple types --- from a C++ programmer's standpoint, this complicates
> > things a lot, but from my standpoint, it gives me a lot more control,
> > avoids templates, and allows me to share the same subroutines for lots of
> > different types -- it just requires a bit of casting.
> >
> > > So now the problem is I believe want to do the following:
> > >
> > > dlistString *strType = NULL;
> > >
> > > *list = New(dlist(strType,&strType->m_link));
> > >
> > > However m_link is private.
> > >
> > > I could easily change the header to fix this problem but I wanted to
> > > ask you first.
> >
> > You did the right thing -- it is better to ask.
> >
> > I think I would do it as follows:
> >
> > int docmd(monitoritem* item, const char* command, dlist *list);
> >
> > dlist *list;
> > list = New(dlist(0,0));
> >
> > docmd(..., ..., list);
> >
> >
> >
> > int docmd(... ..., dlist *list)
> > {
> > list->append(new_dlistString("string"));
> > ...
> > }
> >
> > It works with dlist(0,0) because I "know" that a dlistString has the link
> > as the 0'th item, so no other special initialization is needed.
> >
> > I suppose that I should have made the constructor for a dlist(void) clear
> > the memory allocated. That would have avoided passing any argument, and
> > the code above could have been written more simply:
> >
> > list = New(dlist());
> >
> > which I would ultimately like to convert to:
> >
> > list = new_dlist();
> >
> > which is a lot cleaner IMO, but the new_dlist() subroutine doesn't
> > currently exist.
> >
> > I hope that helps. If not, please don't hesitate to ask.
>
> Yes, This all helps a lot. Next time I will ask you sooner but I guess
> in this case my
> time was not wasted as I did become more familiar with the code trying
> to figure out
> how to allocate memory in the same way bacula does.
I'll let you decide, but I have no problem at all responding to a developer
actively working on a project.
>
> > By the way, a couple of other rather important points to keep in mind:
> >
> > - As you probably know, it is essential that we eliminate all source code
> > licensed under the GPL that is copyrighted by a 3rd party. This is
> > because we need to modify the GPL to permit use with OpenSSL whose
> > license is not GPL compatible, and we cannot modify GPL licenses
> > copyrighted by 3rd parties.
> >
> > - It turns out that you are working on Win32 code where we have a number
> > of files copyrighted by AT&T, so in any of the work you are doing, please
> > be careful not to use any of the AT&T code in new ways. It *must* be
> > rewritten in the near future.
> >
> > - It also turns out that some of the code in the tray-monitor directory
> > is copyrighted by Sun, and another person (the egg... files), and so in
> > the near future, we need to eliminate those files too. It turns out that
> > the egg... code was used to create the tray item, because at the time
> > GTK+ did not have an API interface to the tray, while now it does. So,
> > if you know anything about GTK+ please bear in mind that we would like to
> > eliminate all the egg... calls and replace them by the GTK+ API calls.
>
> I understand that part as well although I only briefly opened these
> files as I knew this stuff would not be used in the win32 code.
Yes, I already imagined so, but I wanted to re-emphasize it, if not for you
for all the other developers.
>
> > - When you are adding Win32 code to Bacula, please take great care not to
> > copy any of the code in any file that has an AT&T copyright without
> > asking me.
>
> I am not sure if I copied any files containing this copyright to this
> point but I will check.
Thanks. Don't hesitate to ask if you have copied something. I can help sort
out the possible solutions.
>
> >A
> > very large percentage of that code is code that I wrote and can thus be
> > used elsewhere but I will have to point it out (just ask this is not a
> > problem). All the rest of the code, we will need to write from scratch.
> >
> > Thanks for working on this :-)
>
> Kern, you are welcome. I thank you for your patience with me on this
> one as in my
> opinion I have taken way too long.
You know, I have been working on this project for more than 7.5 years, so I am
used to projects taking time. What counts the most for me that there is
steady progress (and feedback) in a project. If a project is moving forward,
at some point it will become a reality. What is a pity is when someone
spends a lot of time on something then drops it because he can no longer
afford the time commitment -- I prefer a smaller commitment that is
maintainable.
Best regards,
Kern
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Bacula-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bacula-devel
