请高手点评一下我的代码，无论是挑出bug还是代码风格，或者优化代码方面的建议和意见都非常欢迎，谢谢。
题目是判断一个链表是否循环，请多指教，谢谢。


/* Create a link and judge if it 's a loop link */
#include <stdio.h>

/*link node declaration*/
typedef struct _ar
{
int i;
struct ar *pn;
} ar;

int main(void)
{

ar *p = calloc(1,sizeof(ar)); /* create the first node */
ar *pf = p; /* The pointer to the first node */
ar *ptmp = 0; /* temp pointer */
p-> i=0;
int j=0; /* initialize the first node */
for(j=0; j <30; j++)
{
p-> pn = calloc(1, sizeof(ar)); /* allocate memory space for each node */
p=p-> pn; /* p indicates the next node */
p-> i = j+5;
if(j == 3) /* put down the position of the 4th node */
ptmp=p;
if(j == 29) /* let the last pointer point the 4th node */
p-> pn = ptmp;
}

/* print all the nodes */
ptmp = pf;
for(j=0; j <45; j++)
{
printf( " %d | ", ptmp-> i);
ptmp = ptmp-> pn;
}

/* judge if the link is loop */
ar *p1 = pf;
ar *p2 = pf-> pn;
p2 = p2-> pn;
int count = 0;
while(1)
{
/* the p1 ponter runs by step 1 and the p2 pointer runs by step 2 */
if(p1 == p2)
{
/* The p2 pointer meets the p1 pointer */
printf( "\nThe counter is %d \n ", count);
printf( "The link is loop! and the current position is %d and %d.\n ",p1-> i,p2-> i);
break;
}
if((p1 == NULL) || (p2 == NULL))
{
/*The p2 pointer runs to the last node and the link is not loop*/
printf( "The link is NOT loop!! \n ");
break;
}
count++;
printf( " -%d-%d- ",p1-> i,p2-> i);
p1 = p1-> pn;
p2 = p2-> pn;
p2 = p2-> pn;
}

/* free all the allocated memroy */
p = pf-> pn;
free(pf);
for(j=0; j <30; j++)
{
ptmp = p-> pn;
free(p);
p = ptmp;
}
return 0;
}


------解决方案--------------------

问题很多啊
首先BUG问题
看以下代码
p1 = p1-> pn;
p2 = p2-> pn;
p2 = p2-> pn;
前面两句没有问题, 因为在之前有if((p1 == NULL) || (p2 == NULL)) 然后break了
但最后一句, 然后此时 p2==NULL, 则后出现内存访问错误
也就是说, 当无循环时, 要么奇数个元素, 要么是偶数个元素, 你的程序就会访问空指针
出错概率50%
具体是奇数个还是偶数个, 你自己分析

由于你程序的重点是测试判断一个链表是否循环, 所以, 最好将此部分写从一个函数
命名也很糟糕



[解决办法]
对，变量函数的命名很有问题，最好有意义
[解决办法]
node用template去做吧。。。