Was sind die Fehler in dieser Funktion aus einer Microsoft-Interviewfrage?

Lesezeit: 7 Minuten

Diese Frage wurde mir im schriftlichen Walkin-Interview von MS gestellt:

Finden Sie Fehler im folgenden Programm, das einen neuen String mit zurückgeben soll \n daran angehängt.

char* AddnewlinetoString(char *s)
{
  char buffer[1024];
  strcpy(buffer,s);
  buffer[strlen(s)-1] = '\n';
  return buffer;
}

Ich hatte versucht, mich selbst zu programmieren, und konnte es zum Laufen bringen, indem ich die Puffervariable global machte und hatte buffer[strlen(s)] = '\n'. Wusste aber nicht, dass da noch viele andere Bugs drin sind.

  • Also, welche hast du gesehen?

    – Duffymo

    21. September 2010 um 9:11 Uhr

  • Ist der Code von Windows 95?

    – Alexander C.

    21. September 2010 um 9:14 Uhr

  • Mal sehen, wie Leute aus Java-Schulen diese Frage beantworten 🙂

    – Lukas

    21. September 2010 um 14:41 Uhr

  • Warum wurde diese Frage geschlossen? Es hängt sehr stark mit der Programmierung zusammen.

    – Giri

    21. September 2010 um 17:01 Uhr

  • Es gibt zwei Arten von Programmierern – diejenigen, die ein Problem durchdenken, um alle relevanten Details zu sehen, und diejenigen, die einfach so weiterprobieren, bis es zu funktionieren scheint. Rate mal, in welche Gruppe du gehörst? Ratet mal, nach welcher Art Microsoft sucht?

    – Markieren Sie Lösegeld

    21. September 2010 um 17:49 Uhr

Benutzer-Avatar
codaddict

Ich kann ein paar sehen:

Länge der Eingabezeichenfolge nicht überprüft.

Was wäre, wenn die strlen(s) > 1023? Sie können höchstens eine Schnur der Länge passen 1023 im Puffer.

Überschreiben des letzten Zeichens mit \n

Sie überschreiben das letzte Zeichen mit Zeilenumbruch. Dein \n soll wohin gehen \0 verwendet, und Sie müssen eine neue hinzufügen \0 nach \n

Der variable Puffer ist lokal für die Funktion und Sie geben seine Adresse zurück.

Speicher für den Puffer wird auf dem Stack zugewiesen und sobald die Funktion zurückkehrt, wird dieser Speicher freigegeben.

Ich würde tun:

char* AddnewlinetoString(char *s) {

  size_t buffLen = strlen(s) + 2; // +1 for '\n' +1 for '\0'
  char *buffer = malloc(buffLen); 
  if(!buffer) {
   fprintf(stderr,"Error allocting\n");
   exit(1);
  }
  strcpy(buffer,s);
  buffer[buffLen-2] = '\n';
  buffer[buffLen-1] = 0;
  return buffer;
}

  • Lustigerweise ist die \0 ist wahrscheinlich immer noch da (wenn der erste Fehler es nicht auslöst). Schau nochmal, ist das \0 überschrieben?

    – Muhammad Alkaruri

    21. September 2010 um 9:17 Uhr


  • 2. strlen schließt das Abschlusszeichen nicht in die Länge ein. Der Code ersetzt also höchstens das letzte Zeichen des Strings durch einen Zeilenumbruch, aber die Null bleibt bestehen, wenn sie bereits vorhanden war.

    – chao

    21. September 2010 um 9:18 Uhr

  • @TygerKrash: Der Stapel enthält lokale Werte für die aktuelle Funktion. Sobald die Funktion zurückkehrt, zeigt Ihr Zeiger auf Müll, dh Sie wissen nicht, was dort ist (z. B. können die lokalen Variablen der nächsten Funktion den Bereich überschreiben, auf den Ihr Zeiger zeigt).

    – 3 Elektrologos

    21. September 2010 um 11:32 Uhr

  • Dies wird Speicher verlieren, wenn es wie folgt verwendet wird: str = AddNewlineToString(str);

    – sje397

    21. September 2010 um 14:08 Uhr


  • @sje397: Das ist kein Problem mit der Funktion, vorausgesetzt, es ist ordnungsgemäß dokumentiert, dass der Aufrufer den zurückgegebenen Puffer besitzt und dafür verantwortlich ist, ihn entsprechend freizugeben. Die einzige Alternative besteht darin, mit einem globalen Puffer zu arbeiten, der viel schwerwiegendere Probleme hat. *edit: Vorausgesetzt, dass Sie natürlich mit einer C-ähnlichen Schnittstelle feststecken.

    – Dennis Zickefoose

    21. September 2010 um 16:54 Uhr


  1. Es gibt keine Begrenzung in strcpy, verwenden Sie besser strncpy.
  2. Sie kopieren in den statischen Puffer und geben den Zeiger zurück.

  • Sicherlich ist das kein statischer Puffer?

    – abschalten

    21. September 2010 um 9:12 Uhr

  • Anstatt von strncpy man sollte sicherstellen, dass der Puffer eine geeignete Größe hat. Was nützt es genau, wenn die Eingabe abgeschnitten wird, anstatt dass Zeilenumbrüche angehängt werden? Dies sollte beide Probleme lösen.

    – Besucher

    21. September 2010 um 9:16 Uhr

  • Wenn verfügbar, wäre strlcpy besser.

    – Matteo Italien

    21. September 2010 um 9:22 Uhr

Benutzer-Avatar
fredoverflow

Hier ist eine C++-Version ohne Fehler:

std::string AddnewlinetoString(std::string const& s)
{
    return s + "\n";
}

Und so würde ich das wahrscheinlich in C++0x schreiben:

std::string AddnewlinetoString(std::string s)
{
    return std::move(s += "\n");
}

  • Beantwortet aber nicht die Frage 🙂

    – Philipp Ekberg

    21. September 2010 um 9:23 Uhr

  • Stimmt, aber… Ich glaube nicht, dass der Zweck dieser Übungen darin besteht, eine bessere Version zu finden an sich, sondern um die Feinheiten der Fehler zu verstehen. Betrachten Sie es als Test zum Debuggen der Arbeit Ihrer mittelmäßigen Kollegen und nicht als Test zum Schreiben von neuem Code.

    – Andrzej Doyle

    21. September 2010 um 9:25 Uhr

  • Die Frage ist mit C++ gekennzeichnet, was bedeuten würde, dass dies eine potenziell richtige Antwort ist!

    – Ashleys Gehirn

    21. September 2010 um 12:18 Uhr

  • @Steve: Ich habe Ihren vorgeschlagenen Verweis auf die const-Version hinzugefügt, aber ich mag immer noch die Wert übergeben Version besser, vor allem mit der std::move bearbeitet in 🙂

    – fredoverflow

    21. September 2010 um 14:14 Uhr


  • Zusammenfassend denke ich, dass der beste Weg sowohl für C++03 als auch für C++0x wäre, std::string AddnewlinetoString(std::string s){ s += '\n'; return s; } Zum Parameter: In C++03 würde man bei der Arbeit mit einem Lvalue ohnehin eine Kopie machen, damit nichts verloren geht. Und ein Compiler entfernt die Kopie eines Rvalue. In C++0x erstellen Sie erneut die erforderliche Kopie, aber es wird garantiert, dass der Parameter verschoben wird, wenn ihm ein rvalue gegeben wird. Da geht also nichts verloren. Und dann führen Sie Ihre Manipulation durch und geben dann die Kopie zurück. In C++03 erhalten Sie NRVO und in C++0x erhalten Sie eine implizite Bewegungsrückgabe. (Wieder denke ich.)

    – GManNickG

    22. September 2010 um 18:11 Uhr

Benutzer-Avatar
marmich

Ich würde auch hinzufügen, dass der Name der Methode dem Muster entsprechen sollte und jedes Wort mit einem Großbuchstaben beginnen sollte:

char* AddNewlineToString(char *s)
{
}

p.s. Danke Konrad, ich habe den Methodennamen geändert, wie Sie vorgeschlagen haben

Benutzer-Avatar
äh

3 Dinge

   int len = strlen(s);
   char* buffer = (char*) malloc (len + 2);   // 1
   strcpy(buffer,s);
   buffer[len] = '\n';           // 2 
   buffer[len+1] = '\0';         // 3
   return buffer;

Bearbeiten: Basierend auf Kommentaren

  • Berufung strlen() drei Mal anstelle eines Provisoriums kann zu enttäuschenden Ergebnissen führen.

    – scharfer Zahn

    21. September 2010 um 9:23 Uhr

  • @aeh: strlen zählt das abschließende ‘\0’ nicht, ersetzt also tatsächlich das letzte Zeichen vor dem ‘\0’ und schlägt bei leeren Zeichenfolgen fehl, die nur das Ende enthalten.

    – Sicher

    21. September 2010 um 10:31 Uhr


  • @aeh: Ihre Antwort enthält einen Fehler. Sie geben einfach einen neuen String zurück, wobei das letzte Zeichen von s durch ersetzt wird \n. len ist ein Zeichen zu klein.

    – JeremyP

    21. September 2010 um 10:39 Uhr

  • Sie suchen nicht nach s Sein NULL und für malloc() Versagen.

    – Georg Fritzsche

    23. September 2010 um 23:28 Uhr

  • würden sie auch gerne char* AddnewlinetoString(const char *s)

    – ruslik

    24. September 2010 um 23:45 Uhr

Hier ist eine korrigierte Version (Community-Wiki, falls ich etwas verpasst habe)

// caller must free() returned buffer string!
char* AddnewlinetoString(char *s)
{
  size_t len;
  char * buffer;

  if (s == NULL)
    s = "";

  len = strlen(s);
  buffer = malloc(len+2);
  if (buffer == NULL)
    abort();
  strcpy(buffer,s);
  buffer[len] = '\n';
  buffer[len+1] = 0;
  return buffer;
}

Wie Tony erwähnt, kann s eine gültige Adresse sein, aber immer noch ein fehlerhafter C-String ohne Null-Bytes. Die Funktion könnte so lange lesen, bis sie einen Segfault oder etwas anderes Schreckliches verursacht. Obwohl dies immer noch das idiomatische C ist, bevorzugen die meisten Leute gezählte Zeichenfolgen (statt nullterminierte).

// caller must free() returned buffer string!
char* AddnewlinetoStringN(char *s, size_t len)
{
  char * buffer;

  if (s == NULL)
    s = "";

  buffer = malloc(len+1); // only add 1 byte, since there's no need for the nul
  if (buffer == NULL)
    abort();
  strncpy(buffer,s,len);
  buffer[len] = '\n';
  return buffer;
}

  • Berufung strlen() drei Mal anstelle eines Provisoriums kann zu enttäuschenden Ergebnissen führen.

    – scharfer Zahn

    21. September 2010 um 9:23 Uhr

  • @aeh: strlen zählt das abschließende ‘\0’ nicht, ersetzt also tatsächlich das letzte Zeichen vor dem ‘\0’ und schlägt bei leeren Zeichenfolgen fehl, die nur das Ende enthalten.

    – Sicher

    21. September 2010 um 10:31 Uhr


  • @aeh: Ihre Antwort enthält einen Fehler. Sie geben einfach einen neuen String zurück, wobei das letzte Zeichen von s durch ersetzt wird \n. len ist ein Zeichen zu klein.

    – JeremyP

    21. September 2010 um 10:39 Uhr

  • Sie suchen nicht nach s Sein NULL und für malloc() Versagen.

    – Georg Fritzsche

    23. September 2010 um 23:28 Uhr

  • würden sie auch gerne char* AddnewlinetoString(const char *s)

    – ruslik

    24. September 2010 um 23:45 Uhr

Benutzer-Avatar
Miguel Vieira

Das Hauptproblem bei diesem Code ist, dass er anfällig für a ist Stack-Pufferüberlauf Ausbeuten. Es ist ein klassisches Beispiel.

Grundsätzlich kann das Eingabezeichen* länger als 1024 Bytes gemacht werden; Diese zusätzlichen Bytes überschreiben dann den Stack, sodass ein Angreifer den Rückgabezeiger der Funktion so ändern kann, dass er auf seinen bösartigen Code zeigt. Ihr Programm führt dann unwissentlich den Schadcode aus.

Es ist zu erwarten, dass Microsoft sich sehr um diese Art von Exploits kümmert, da der Code Red Worm bekanntermaßen im Jahr 2001 einen Stapelpufferüberlauf verwendete, um Hunderttausende von Computern anzugreifen, auf denen IIS-Webserversoftware ausgeführt wurde.

1354460cookie-checkWas sind die Fehler in dieser Funktion aus einer Microsoft-Interviewfrage?

This website is using cookies to improve the user-friendliness. You agree by using the website further.

Privacy policy