我的连接函数mystrcat(char*,char*,char*)有什么问题？

Question

我最近接受采访,要求写mystrcat(*s1, *s2, *s3)在那里s1,并s2在源字符串和串联的结果给出s3.我被告知,不要担心内存分配s3和假设s1,s2并且不是空/无效字符串.所以我写了下面的蹩脚(粗)程序.我被告知有问题s3或出现问题s3.你能告诉它它是什么/可能是什么？

void mystrcat(char *s1, char *s2, char *s3)
{
    if (! (s1 || s2 || s3)) return; // one/more pointers are invalid

    // copy string s1 into s3
    while(*s1) {
      *s3 = *s1;
      s1++;
      s3++;
    }

    // concatenate string s2 into s3
    while(*s2) {
      *s3 = *s2;
      s2++;
      s3++;
    }

    *s3 = '\0';
}

你能告诉我这里有什么问题吗？做什么会更专业呢？

Answer 1

if (! (s1 || s2 || s3) return; // one/more pointers are invalid

应该

if ((!s1) || (!s2) || (!s3)) return;

Answer 2

两个可能的点

首先,您被告知输入和输出指向有效字符串,因此可以说不需要测试有效性.如果需要,你应该吵闹失败.更好的是:

 void mystrcat(char *s1, char *s2, char *s3)
    {
        ASSERT( s1 );
        ASSERT( s2 );
        ASSERT( s3 );
        ....

然后你基本上写了strcat/strcpy,你可以重用它们:

void mystrcat(char *s1, char *s2, char *s3)
{
    strcpy( s3, s1 );
    strcat( s3, s2 );
}

如果我正在采访你以外的其他任何事情,我会特别指出你指出我指定的mystrcat界面是非常精心设计的,并提供了如何改进它的细节.

Answer 3

这是我的评论

• 应该键入s1和s2,const char*因为您无意修改它们.
• 你没有验证s3有足够的分配空间来包含s1和s2的组合长度
• 当用户传入错误数据(NULL值)时,您将无声地失败.这并不好,因为呼叫者无法区分成功和失败的呼叫
• 验证s1,s2和s3的方法不为null是不正确的(Igor没错)

我希望你在面试中问我或自己回答的问题

• 我应该如何表达未能复制字符串？
• 你想要一个精确的strcat克隆还是更现代的一个并且做更好的参数验证？
• 我可以使用其他标准str函数来完成答案吗？