Graphviz Issue Tracker
Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0001634graphvizGraph Librariespublic2009-06-30 13:462011-04-28 04:03
ReporterMarek Elias 
Assigned Tonorth 
PlatformOS*-*-OS Version
Summary0001634: cgraph: segfault when specifying graph disciplines

I tried this code:
struct Agdisc_s disc = { NULL, NULL, NULL };
Agraph_t *G = agread(stdin, &disc);

which resulted in segfault before reading any input.
Another example:

struct Agdisc_s disc = { NULL, NULL, NULL };
Agraph_t *G = agopen("G", Agundirected, &disc);
agconcat(G, stdin, &disc);

agopen was ok. It segfaulted in agconcat. Here is the backtrace for
graphviz 2.24.0 (for 2.22.2 it was similar):

#0 0x08058357 in aag_get_next_buffer () at lex.aag.c:1362
0000001 0x08057f92 in aaglex () at lex.aag.c:1204
0000002 0x080521c5 in aagparse () at
0000003 0x08053702 in agconcat (g=0x0, chan=0x4645c420, disc=0xbfbb7b7c)
   at ../../lib/cgraph/grammar.y:537
0000004 0x0805373c in agread (fp=0x4645c420, disc=0xbfbb7b7c)
   at ../../lib/cgraph/grammar.y:541
0000005 0x080495cd in make_in (f_in=0x4645c420) at visual_mwpm.c:47
0000006 0x0804956d in main (argc=1, argv=0xbfbb7c64) at visual_mwpm.c:38

Am I doing something wrong?
Additional Information

The discipline functions are not defined (NULL).

What are you trying to do here?

Following up on Stephen's response, if you want the default disciplines, do

   agopen("G", Agundirected, NULL);

If you supply a non-NULL third argument, all of the pointers have to be filled in.

This is what I am trying to do:
    struct Agiddisc_s my_id_disc = { NULL, my_id_map, NULL, NULL,
        NULL, NULL };
    struct Agdisc_s disc = { NULL, &my_id_disc, NULL };
    Agraph_t *G = agread(f_in, &disc);
(I want only one custom discipline and in this case it segfaults in the
same manner)

This is what the manual states:
    struct Agdisc_s { /* user's discipline */
        Agmemdisc_t *mem;
        Agiddisc_t *id;
        Agiodisc_t *io;
    } ;
    A default discipline is supplied when NIL is given for any
    of these fields.

And this is, what graph.c:static Agclos_t *agclos(Agdisc_t * proto)
does and it persuades me that manual should be probably right:
    memdisc = ((proto && proto->mem) ? proto->mem : &AgMemDisc);
    rv-> = ((proto && proto->id) ? proto->id : &AgIdDisc);
    rv-> = ((proto && proto->io) ? proto->io : &AgIoDisc);

so what would be the recommended way to do this?

More generally: I have two processes. First reads a graph adds some
nodes and edges and writes it into a file. The second read graph from
that file and makes something different with it. I need:
 * uniquely identify nodes created by first process
 * uniquely identify edges in multigraph (I have multiple edges between
   same nodes).

Well, if we said that, then it's a bug if that fails.

We'll have to look into it further.

I'm reading Stephen's mind here, but I believe the intent was to treat the specific disciplines as units, since
the functions share state and provide a protocol. It doesn't make sense to mix and match the component parts.
That would be like providing your own alloc in the memory discipline, and then relying on the default free function.

At the top level, though, if the Agdisc_t* pointer is null, or one of its fields, the default discipline can be stuck
in as a unit. This is what is currently done, and what the documentation describes.

So, if you want to use your own id discipline, you need to fill in all fields of Agiddisc_s. These can be stubs or,
if you want to be daring, you could grab the function pointers from the AgIdDisc.

Right, Emden explained it pretty well.
If you read the code in graphviz2/lib/agraph/id.c
(or cgraph/id.c which is basically the same thing)
it's pretty clear that we're abusing the relationship
between strings and longs, so we can use string
pointers for user IDs, and some other values which
are always odd, for internal anonymous IDs.
You couldn't just redefine the mapping function
and expect it to work.

I guess it would be nice for somebody to write
a kind of more-expensive-but-generic ID mapper,
using say a "cdt" dictionary to store the values.
Or use your own data structures.

Perhaps John Ellson has already done something like this?

Otherwise, at least it's open source :-)
TagsNo tags attached.
STATUS-COMMENTFixed (29 Jun 2009)
VERSION     2.24
Attached Files

- Relationships

-  Notes
There are no notes attached to this issue.

- Issue History
Date Modified Username Field Change
2011-04-28 04:03 user1 New Issue
2011-04-28 04:03 user1 Assigned To => Stephen North

MantisBT 1.2.5[^]
Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker