[Home]

Summary:ASTERISK-05674: [patch] System name option and variable
Reporter:Manuel Guesdon (mguesdon)Labels:
Date Opened:2005-11-22 10:46:15.000-0600Date Closed:2008-01-15 16:47:05.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/Configuration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) diffurn
( 1) patch-20051123-131800.diff
( 2) patch-20051126-125500.diff
Description:Here is a patch to have systemname variable.
Config:
in asterisk.conf:
[options]
systemname = MySystemName

channel uniqueid is prefix by the system name (like in patch 'bristuff') if any. (example: MySystemName-1132669009.0)
It add a variable ${SYSTEMNAME}



Comments:By: Olle Johansson (oej) 2005-11-22 11:00:49.000-0600

Good patch, but formatting is not correct. Make sure you indent properly according to the bug guidelines.

Also, I would like to see this as a header in manager.

Thank you for contributing to Asterisk!
By: Manuel Guesdon (mguesdon) 2005-11-22 11:24:07.000-0600

Here it is formatted with diff -urN
About manager header, where is it generated ?
By: Donny Kavanagh (donnyk) 2005-11-22 13:16:44.000-0600

manager.c line 1394 in my few day old cvs.

ast_cli(s->fd, "Asterisk Call Manager/1.0\r\n");
By: Russell Bryant (russell) 2005-11-22 15:13:51.000-0600

I think that this is a much better approach than the one I used in ASTERISK-4861.  :)

A couple of notes:

1) You still appear to have some tabs vs. spaces issues in your patch.  Please only use tabs as indentation.

2) Can you add this new option to the doc/README.asterisk.conf?

Thanks!
By: Donny Kavanagh (donnyk) 2005-11-22 15:24:32.000-0600

On a side note, should this now say Asterisk Call Manager/1.2? or is the protocol still version 1.0?
By: Russell Bryant (russell) 2005-11-22 19:49:13.000-0600

A couple more code comments:

1) Please use "if (!ast_strlen_zero(ast_config_AST_SYSTEM_NAME))" instead of "if (*ast_config_AST_SYSTEM_NAME)".

2) Please use ast_copy_string instead of strncpy.

A lot of these issues are covered in the doc/CODING-GUIDELINES document.  :)
By: Russell Bryant (russell) 2005-11-23 00:37:52.000-0600

As for adding this somewhere in the manager interface, I don't think it is necessary to have that as a part of this patch.  There are multiple places where the system name could be used.  We can handle them as seperate issues.
By: Manuel Guesdon (mguesdon) 2005-11-23 06:33:32.000-0600

Done. Here is the new patch.
Sorry for guidelines, I used to work on GNUstep and guidelines are differents :-)
It doesn't include changes for manager as I don't know this part and I think it's better to do it in a separate patch.
By: twisted (twisted) 2005-11-23 23:23:51.000-0600

This may be a silly question, but what would be the benefit of this rather than determining it from the hostname?
By: Manuel Guesdon (mguesdon) 2005-11-24 03:13:04.000-0600

If we take full qualified hostname like "mylonghostname.mylongdomain.com", its size is greater than cdr unique field size.
If we take only "myhostname", we may have multiple one (for example myhostname.customer1.com and myhostname.customer2.com).
If you also want to use it for configuration things, it's better to have the possibility to choose exactly the name you want.
Also, for some reason you may can't change the hostname (for license reason for example) but want to have a specific systemname in asterisk....
By: Manuel Guesdon (mguesdon) 2005-11-26 05:57:33.000-0600

Uploaded  patch-20051126-125500.diff to xfix a stupid error: ${SYSTEMNAME} was tested/replaced only when there was a channel.
By: Russell Bryant (russell) 2005-12-05 15:22:24.000-0600

I think we should add a note to README.asterisk.conf about the size limitation of the uniqueid field.  If someone uses a systemname that is too long, they will lose the unique integer at the end.
By: Tilghman Lesher (tilghman) 2005-12-05 15:45:53.000-0600

That might be a good reason to put the systemname at the end, in this patch.
By: Manuel Guesdon (mguesdon) 2005-12-05 16:03:55.000-0600

I'd prefer having it as prefix, this seems more 'natural' to me.
A solution could be to take only n first characters if it is too long and emit a warning in this case.
BTW, is there a reason to limit some fields length to 20 or 32 characters (like uniqueid and account code) ? Why not using a 64 characters length. This won't consume excessive memory if we consider ast_channel or ast_cdr structure size...
By: Russell Bryant (russell) 2005-12-05 16:10:25.000-0600

I think it's fine to only take the first 'n' characters and print a warning if it is too long.  Of course, this should be done when it's read from the config file, and not on every call to ast_channel_alloc.  :)

As far as the length of the uniqueid goes, I'm against changing its size by default, size the majority of users would not be affected by this.  However, it would be a good idea to add a define that could be changed if a user chooses to do so.  This could be documented along with the new option in README.asterisk.conf.

in include/asterisk/channel.h:

#define MAX_UNIQUEID         32

struct ast_channel {
  ...
  char uniqueid[MAX_UNIQUEID];
  ...
}
By: Matt O'Gorman (mogorman) 2006-01-17 14:49:47.000-0600

mguesdon have you had a chance to address these issues yet?
By: Olle Johansson (oej) 2006-01-30 14:22:36.000-0600

*** Reminder from housekeeping ***
By: Kevin P. Fleming (kpfleming) 2006-02-14 16:43:13.000-0600

Committed to trunk with minor mods to deal with changes in the code since the patch was updated. Also converted chan->uniqueid into a stringfield so the size is no longer an issue.
By: Digium Subversion (svnbot) 2008-01-15 16:47:05.000-0600

Repository: asterisk
Revision: 10088

U   trunk/asterisk.c
U   trunk/channel.c
U   trunk/channels/chan_agent.c
U   trunk/doc/asterisk-conf.txt
U   trunk/doc/channelvariables.txt
U   trunk/include/asterisk/channel.h
U   trunk/include/asterisk.h
U   trunk/pbx.c

------------------------------------------------------------------------
r10088 | kpfleming | 2008-01-15 16:47:05 -0600 (Tue, 15 Jan 2008) | 3 lines

add 'systemname' option to prefix channel unique IDs with (issue ASTERISK-5674)
convert chan->uniqueid to a stringfield from a fixed-size buffer

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=10088