Hello John,

On Tuesday 07 August 2007 13:44, John Drescher wrote:
> Kern,
> Here is my problem. I have been looking through the code deciding how best
> to approach this task and I have decided to convert src/tray-monitor/tray-
> monitor.c such that the win32 and unix tray-monitor use as much as the
> common functionality as they can. I initially used ifdef to not compile the
> gtk stuff in the win32 build so that I eventually as able to build a win32
> bacula-tray-monitor.exe that executed (although it had no output). 

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.

> 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 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; 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");

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.

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.

- 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.  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 :-)

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

Reply via email to